-
Notifications
You must be signed in to change notification settings - Fork 791
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 cleanup #437
Test cleanup #437
Conversation
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.
Thanks for this, I'm totally on-board with cleaning the tests.
84a3f3b
to
b5e1b47
Compare
Just x-checked, the decrease in test coverage is from the So no actual coverage decrease and we can merge if everything else is ok. |
… due to DEPRECATION status)
…d with info msg pointing to more specific commands
b5e1b47
to
ed5041f
Compare
Already reviewed by @s1na, no production code touched, will merge. |
This half-cleaned tests directory together with the outdated
runAll()
test run function intester.js
is for quite some time some reoccurring source of confusion, latest during the work on #406.This PR does some cleanup here:
Separate
manual
directory with the manualconstantionpleSstoreTest
test fileThis was once written by Vinay since the official test suite was not yet available. Tests still pass so would be a bit a pity to throw away. For the current situation this extra
manual
command might be a bit too much. Reasoning would be nevertheless that there is then a place for similar tests in the future if necessary and things have some place then and there will be some higher chance that structure won't get bloated on stuff like this again.Update: Tests deleted after discussion.
Included
hooked.js
into the API test suiteThese tests are still working. We can remove/adopt depending on what we do with this part of the API. For now the code is still there so this fits better in the API directory than having some (too prominent) separate place.
Integrated genesis hash test into the API test suite
This test was also floating around, not used for quite some time but still working. Have adopted and added to the API test suite. Genesis functionality of
StateManager
is also not sure to be kept on final extracted version. Nevertheless for the moment it's still there and used.Replaced outdated runAll() test run code with info message
When
npm run test
is run this is now showing a Command-not-used message. The code within the functionrunAll()
is commented out. This was/is some major source of confusion for newcomers who tend to integrate their new tests there.Update: Function deleted after discussion.