Proposal for unit testing on test_framework functions:
unittest library. Don’t use test_framework to test itself.test_runner.py. To include more Test Framework tests, add the filename to the list TEST_FRAMEWORK_MODULES. Don’t add new files or change the list of accepted script prefixes.Makes these changes for bn2vch (followup to this comment).
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
No conflicts as of last run.
23@@ -24,6 +24,10 @@
24 import tempfile
25 import re
26 import logging
27+import unittest
28+import test_framework.script
It seems a shame to have to list the modules out multiple times. If you use the loadTestsFromName() method, you can avoid having to import the modules:
0TEST_FRAMEWORK_MODULES = [
1 "script",
2 "key",
3 "messages",
4]
5...
6 # Test Framework Tests
7 for module in TEST_FRAMEWORK_MODULES:
8 suite = unittest.TestLoader().loadTestsFromName("test_framework.{}".format(module))
9 unittest.TextTestRunner(verbosity=1).run(suite)
If you wanted to be really clever, you could just search all the modules for which ones import unittest and then automatically pick them up in the test runner, so they don’t need to be listed out at all. I don’t think that’s necessary in this PR, but you might want to consider it in a follow-up.
392@@ -384,6 +393,11 @@ def run_tests(*, test_list, src_dir, build_dir, tmpdir, jobs=1, enable_coverage=
393 if os.path.isdir(cache_dir):
394 print("%sWARNING!%s There is a cache directory here: %s. If tests fail unexpectedly, try deleting the cache directory." % (BOLD[1], BOLD[0], cache_dir))
395
396+ # Test Framework Tests
397+ for module in TEST_FRAMEWORK_MODULES:
398+ suite = unittest.TestLoader().loadTestsFromModule(module)
399+ unittest.TextTestRunner(verbosity=2).run(suite)
TestCase.run() is called, so I made it more concise by grouping all the tests into one TestSuite and calling run() just once instead of doing them each individually. These tests would probably all be very small so I figured it would be alright. On success, it’s 4 lines. On fail, it’s more verbose and quits early. Hopefully this is ok?
Concept and approach ACK. Thanks for tackling this, @gzhao408 .
I left a couple of comments inline. A few more ideas:
Concept ACK
Thanks for tackling this. Great initiative! :)
Test the test_framework, but don't use test_framework objects or functions to test itself
Use python unittest library and put test_framework's unit tests inside their respective files
Add the filename to TEST_FRAMEWORK_MODULES in test_runner
Aggregate all test_framework tests into one TestSuite to run before the functional tests in test_runner
Delete framework_test_script, move test_bn2vch to script.py and add to TEST_FRAMEWORK_MODULES in test_runner
Ready for Review if anyone wants to take a look :) addressed jnewbery’s comments.
It looks like travis failed on wallet_bumpfee.py on the last push. I just passed it locally and can’t figure out why it’s failing, so I’m having it run again. Will look again in a bit…
Tested ACK de8905adf204c42bba810802f82b98f7b3dd26dc. Great stuff gzhao408 . Thanks for this!
For your next PR, try to summarize the why instead of the how in your commit log. People can look at the code to see what changes you’ve made, such as which files you’ve deleted. The log should summarize and give motivation.
Thanks so much for the review @jnewbery!
For your next PR, try to summarize the why instead of the how in your commit log. People can look at the code to see what changes you’ve made, such as which files you’ve deleted. The log should summarize and give motivation.
Ah, gotcha! Thanks for the feedback :) will do.
Labels
Tests