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

Add support to test a custom state test #202

Merged
merged 6 commits into from
Oct 9, 2017
Merged

Conversation

jwasinger
Copy link
Contributor

@jwasinger jwasinger commented Sep 14, 2017

This PR adds the ability to test against custom-generated state tests. The argument --customStateTest is used to specify the relative path of a file containing a custom test.

This PR is necessary for implementing trace equivalence testing and fuzz testing with EthereumjsVM.

Depends on changes in https://github.com/ethereumjs/ethereumjs-testing/tree/test-from-source

@@ -72,6 +72,7 @@ function runTestCase (testData, t, cb) {
}

module.exports = function runStateTest (options, testData, t, cb) {
debugger
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this some new ES* magic?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since testState is being run with node inspect this acts as a breakpoint for the debugger.

@jwasinger jwasinger force-pushed the state-test-from-source branch 2 times, most recently from 688fbf7 to 97558a8 Compare September 17, 2017 23:21
@holgerd77
Copy link
Member

I've now looked 2-3 times over this PR and I still have difficulties to get the essence of it respectively grab what the PR wants to achieve. Can you come up with a practical example? Is this something different that can be achieved with either the new testsPath flag (put your custom test in a separate folder) or the existing getSingleFile method in ethereumjs-testing?

@jwasinger jwasinger requested a review from cdetrio September 21, 2017 22:32
@jwasinger jwasinger force-pushed the state-test-from-source branch from 97558a8 to 2269a7f Compare September 21, 2017 22:37
@jwasinger
Copy link
Contributor Author

jwasinger commented Sep 22, 2017

Yes. This is necessary for fuzz testing and for doing trace equivalence testing. Even with the current CLI options, in order to run a state test, it has to be located under the tests file tree.

@holgerd77
Copy link
Member

Sorry, I'm not really getting it, can' you just make a directory ../MyFuzzTests/tests/fuzztest1.json and the use the new --testsPath option with ../MyFuzzTests/ - and eventually with --test='fuzztest1'?

@jwasinger jwasinger force-pushed the state-test-from-source branch from 2269a7f to 4a76abd Compare September 27, 2017 05:18
@holgerd77
Copy link
Member

Can you give a short comment on my last question?

@jwasinger
Copy link
Contributor Author

jwasinger commented Oct 6, 2017

I changed the command line argument name to 'customStateTest'. We could potentially do what you are suggesting. But IMO it would be easier to have a separate command line option to directly pass the test to tester.js.

The alternative would be to modify this code to move the state test file to the correct location (as opposed to passing it via cli argument):
https://github.com/jwasinger/evmlab/blob/b3f8f48591de6320bba78230290bcf24968bfc02/trace_statetests.py#L358-L369

I'll see if I can do some modifications to get it working in this manner.

@holgerd77 holgerd77 force-pushed the state-test-from-source branch from 9ec6821 to ff1f674 Compare October 9, 2017 11:47
Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I updated this with switching back to the master branch ethereumjs-testing version and updated the README to use the latest parameter naming.

@holgerd77 holgerd77 merged commit 2fe8fed into master Oct 9, 2017
@axic axic deleted the state-test-from-source branch October 12, 2017 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants