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

[test] Extraction of 292 tests from SolidityEndToEndTest.cpp #8464

Merged
merged 2 commits into from
Mar 19, 2020

Conversation

aarlt
Copy link
Member

@aarlt aarlt commented Mar 9, 2020

  • verified correctness of extracted tests with scripts/end-to-end/verify-testcases.py

@aarlt
Copy link
Member Author

aarlt commented Mar 9, 2020

./verify-testcases.py
addmod_mulmod_zero
    kind='call' parameter='1212121212121212121212121212120000000012' input='' output='' value='' result='4'
    kind='call' parameter='1212121212121212121212121212120000000012' input='' output='' value='' result='2'
                                                                                                         |
292 test-cases -  1  mismatche(s)

The cause of the mismatch was a typo within the original boost test-case:

ABI_CHECK(callContractFunction("f(uint)", 0), encodeArgs());
ABI_CHECK(callContractFunction("g(uint)", 0), encodeArgs());
ABI_CHECK(callContractFunction("h()"), encodeArgs(2));

signatures need to be f(uint256) and g(uint256)

@aarlt
Copy link
Member Author

aarlt commented Mar 9, 2020

@chriseth This PR is based on the verification of the original test traces (created with./soltest --log_level=test_suite -t SolidityEndToEndTest/ -- --no-smt --show-messages) vs. the traces created by the extracted tests (created with ./soltest --log_level=test_suite -t semanticTests/extracted -- --no-smt --show-messages). For the comparation of these traces scripts/end-to-end/verify-testcases.py was used.

I also added the scripts/end-to-end/remove-testcases.py script here. From my point of view someone can just add newly extracted files (manually or automated) to semanticTests/extracted, create the traces, run the verifier and if everything was ok, execute scripts/end-to-end/remove-testcases.py > SolidityEndToEndTest.cpp.

@ekpyron
Copy link
Member

ekpyron commented Mar 10, 2020

@aarlt Could you add a command line option like -i to remove-testcases.py that puts it in "interactive mode"? I.e. for each test case it outputs the whole BOOST_AUTO_TEST_CASE(...) {...} part it is going to remove together with the corresponding extracted file and waits for user input to continue.
That way I could quickly step through the 292 test cases and confirm visually for a second or so that it looks ok. That plus looking at the traces I would consider enough to approve this.

@ekpyron
Copy link
Member

ekpyron commented Mar 10, 2020

@aarlt Also what about the plan to also add the metadata to the traces and to compare it (excluding the sources)?

@aarlt
Copy link
Member Author

aarlt commented Mar 10, 2020

@aarlt Could you add a command line option like -i to remove-testcases.py that puts it in "interactive mode"? I.e. for each test case it outputs the whole BOOST_AUTO_TEST_CASE(...) {...} part it is going to remove together with the corresponding extracted file and waits for user input to continue.
That way I could quickly step through the 292 test cases and confirm visually for a second or so that it looks ok. That plus looking at the traces I would consider enough to approve this.

Thats a really nice idea!

@aarlt
Copy link
Member Author

aarlt commented Mar 10, 2020

@aarlt Also what about the plan to also add the metadata to the traces and to compare it (excluding the sources)?

I will do that today. I will create a new PR for this.

@chriseth
Copy link
Contributor

With the single step thing, this looks good, I would say. Can you do it in a way that it displays a diff somehow? At best with "inline diff", ignoring whitespace and marking intra-line changes bold

@aarlt aarlt force-pushed the end-to-end-verified branch from 803c97e to b9b5511 Compare March 11, 2020 01:11
@aarlt
Copy link
Member Author

aarlt commented Mar 11, 2020

@ekpyron @chriseth I added a simple interactive mode to view the extracted test-cases. I was not really able to find a nice way to visualise with diff. I hope that the simple visualisation I've implemented here helps a bit.

usage: ./remove-testcases.py [-i] [-f <full path to SolidityEndToEndTest.cpp>]

You should point with -f to a location of SolidityEndToEndTest.cpp file of the develop branch.

Copy link
Member

@ekpyron ekpyron left a comment

Choose a reason for hiding this comment

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

Two initial comments for the scripts - I'll have a look at an interactive run.

@ekpyron
Copy link
Member

ekpyron commented Mar 11, 2020

Can you avoid requiring a return confirmation for the (y/n) answer? So that hitting a y or n key once suffices without further confirmation?

@aarlt aarlt force-pushed the end-to-end-verified branch 4 times, most recently from 7c6db31 to 494ad7c Compare March 12, 2020 00:01
@aarlt aarlt force-pushed the end-to-end-verified branch 2 times, most recently from ce28e2c to b0655d1 Compare March 16, 2020 18:37
@aarlt
Copy link
Member Author

aarlt commented Mar 16, 2020

@chriseth @ekpyron I added a simple bash script create_traces.sh that will create the needed trace-files for verify-testcases.py. verify-testcases.py will compare the metadata, all create and call messages. Basically it just compares the traces of the original end-to-end tests (SolidityEndToEndTest.cpp) with the traces that where generated by the extracted tests. Just execute ./verify-testcases.py.

To be able to review the extracted test-cases the remove-testcases.py script can be used in interactive mode. You will need to point to a SolidityEndToEndTest.cpp file where the tests where not yet extracted, e.g. use the current develop branch. If you use create_traces.sh, you can use the already checked out develop branch to point to SolidityEndToEndTest.cpp with ./remove-testcases.py -i -f solidity-develop/test/libsolidity/SolidityEndToEndTest.cpp.

The diff-tool used by the remove-testcases.py script can be defined by setting the DIFF environment variable.

@chriseth
Copy link
Contributor

Scripts look ok, will take a look at the tests now.

@aarlt aarlt force-pushed the end-to-end-verified branch from b0655d1 to df8e762 Compare March 18, 2020 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants