-
Notifications
You must be signed in to change notification settings - Fork 677
Snapshot API saves and restores time deltas. #2
Conversation
This offers better support for unit testing.
Is there a reason not to merge these changes? |
Please merge, much needed |
Add tests for leading zeros in hex values
@scnale sorry it has taken us so long to circle 'round to this PR! Unfortunately, I don't think it's working. I've pushed a (failing) test to this PR which demonstrates how I think it's meant to work. Can you please either update the test to correct my misunderstanding, or update your solution to make the test pass? |
I don't know why, but the new test isn't showing in the latest travis build. I promise it's there, and if you run it locally it fails. |
Edit: Nevermind, I thought it was comparing timestamps. I will look into it once I get it setup. |
@scnale it's been quite a while - no pressure, but do you intend to fix up the test failures? If not, I'll take it on if I have time, or if I don't I'll just close this PR and flag the issue as |
Please fix/merge, much needed to write real tests. Thanks. |
@ouziel-slama, per comments above, the changes in this PR don't work as intended. Since I haven't heard back from @scnale in quite a while I'll close this to avoid confusion. If you'd like to fork @scnale's changes and submit a new PR yourself, I strongly encourage it! |
@benjamincburns and @scnale the fix works as intended. There is a mistake in the new test case, where revert should revert to snapshots[1]. Line Trace with current test implementation
The current test assert that |
But... but I wrote that test case! 😊 Thanks so much for pointing this out, @PhABC - I'll have a look! |
I merged this PR, but forgot the |
test/time_adjust.js
Outdated
// Adjust time | ||
web3.eth.getBlock('latest', function(err, block){ | ||
if(err) return done(err) | ||
var previousBlockTime = block.timestamp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused var, fyi
Merge coverage-20180803 into coverage
When running tests, the snapshot API wasn't saving the time adjustments made through the
evm_timeIncrease
JSON-RPC command. I added the code necessary to save and restore the appropriate state but I'm not sure this is the best way to do it. According to my own tests it seems to be working.