-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Share config between util and functional tests #10331
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
Share config between util and functional tests #10331
Conversation
The functional tests and util tests both require a config file that is generated by ./configure. This commit merges those two config files into a single configuration file that can be shared by both tests. The config from config.ini is put into a Namespace object to maintain the interface with bctest.py. A future commit could change this interface to use a dictionary instead of a namespace.
1c05373 to
3fe56f2
Compare
|
I have tested this locally, both for an in-tree and out-of-tree build. |
jimmysong
left a comment
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.
utACK 3fe56f2860a2226b1295ffaded5817098ac5898d
Figuring out exactly how to test this
|
Tested ACK 3fe56f2860a2226b1295ffaded5817098ac5898d |
|
Do we need to mention the Python 3 requirement somewhere? |
So the difference is that 'make check' now requires Python 3? |
Correct. That would also be the case for #10044 (see discussion here: https://botbot.me/freenode/bitcoin-core-dev/2017-03-16/?msg=82548466&page=5) |
|
Concept ACK |
TheBlueMatt
left a comment
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.
Otherwise looks good.
test/util/bctest.py
Outdated
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.
This block didnt make the move.
|
Thanks @TheBlueMatt . Block restored in fixup commit. I'll squash when this is ready for merge. |
|
Please squash the two fixup commits. |
bctest.py is only used as an import by bitcoin-util-test.py. There's no value in keeping it as a separate module, so let's merge them into a single module to keep building and packaging simpler. bitcoin-test-util is importable as a module, so if any future modules really want to import the code from bctest.py, they can import bitcoin-test-util and call the bctest functions by name.
23174d3 to
8ad5bde
Compare
|
fixup commits squashed |
|
utACK 8ad5bde |
8ad5bde Merge bctest.py into bitcoin-util-test.py (John Newbery) 95836c5 Use shared config file for functional and util tests (John Newbery) 89fcd35 Use an .ini config file for environment vars in bitcoin-util-test.py (John Newbery) e9265df Change help_text in bitcoin-util-test.py to a docstring. (John Newbery) ce58e93 Change bitcoin-util-test.py to use Python3 (John Newbery) Tree-SHA512: 66dab0b4a8546aee0dfaef134a165f1447aff4c0ec335754bbc7d9e55909721c62f09cdbf4b22d02ac1fcd5a9b66780f91e1cc4d8687fae7288cc9072a23a78f
8ad5bde Merge bctest.py into bitcoin-util-test.py (John Newbery) 95836c5 Use shared config file for functional and util tests (John Newbery) 89fcd35 Use an .ini config file for environment vars in bitcoin-util-test.py (John Newbery) e9265df Change help_text in bitcoin-util-test.py to a docstring. (John Newbery) ce58e93 Change bitcoin-util-test.py to use Python3 (John Newbery) Tree-SHA512: 66dab0b4a8546aee0dfaef134a165f1447aff4c0ec335754bbc7d9e55909721c62f09cdbf4b22d02ac1fcd5a9b66780f91e1cc4d8687fae7288cc9072a23a78f
The functional tests and util tests both require a config file that is
generated by ./configure. This PR merges those two config
files into a single configuration file that can be shared by both tests.
This PR also merges bctest.py and bitcoin-util-test.py into a single module.
bctest.py is only used as an import by bitcoin-util-test.py. There's no
value in keeping it as a separate module, so this PR merges them into a
single module to keep building and packaging simpler.
Note - this bumps the required python version for bitcoin-util-test.py to python 3.