Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reading trace file allFiredEvents fails silently on large files. #119

Closed
rudolfix opened this issue Sep 29, 2017 · 4 comments · Fixed by #122
Closed

Reading trace file allFiredEvents fails silently on large files. #119

rudolfix opened this issue Sep 29, 2017 · 4 comments · Fixed by #122
Labels

Comments

@rudolfix
Copy link
Contributor

readFileSync will fail on files larger than ~260MB as described here.
nodejs/node#9489
This will make solidity to fail with cryptic message when calling toString on the readFileSync results.
We encountered this problem due to (relatively) large number of tests we run on our code base.

Solution: use stream to read line by line as in this commit Neufund@9a041d7
If maintainers are interested I can provide PR.

@cgewecke cgewecke added the bug label Sep 29, 2017
@cgewecke
Copy link
Member

Whoa! That's a lot of events! Thanks for reporting.

Yes we'll happily take as a PR! Thank you @rudolfix.

@cgewecke
Copy link
Member

cgewecke commented Oct 2, 2017

@rudolfix Absolutely no pressure here but is your preference to open a PR or is it more convenient for you if we just steal this? With attribution of course.

If PR that's great, and take as long as you like :)

@rudolfix
Copy link
Contributor Author

rudolfix commented Oct 2, 2017

Do you have gitter channel? All tests are passing except app test which throws on timeout

1) app "before all" hook:
     Error: Timeout of 60000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

I run it with npm run test. I'd like all tests passing before I push PR

@cgewecke
Copy link
Member

cgewecke commented Oct 2, 2017

@rudolfix I DM'd you there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants