Skip to content

Conversation

@jnewbery
Copy link
Contributor

@jnewbery jnewbery commented May 3, 2017

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.

jnewbery added 4 commits May 3, 2017 14:14
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.
@jnewbery jnewbery force-pushed the shared_util_function_test_config branch from 1c05373 to 3fe56f2 Compare May 3, 2017 18:56
@jnewbery
Copy link
Contributor Author

jnewbery commented May 3, 2017

I have tested this locally, both for an in-tree and out-of-tree build.

Copy link
Contributor

@jimmysong jimmysong left a 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

@jimmysong
Copy link
Contributor

Tested ACK 3fe56f2860a2226b1295ffaded5817098ac5898d

@fanquake
Copy link
Member

Do we need to mention the Python 3 requirement somewhere?

@laanwj
Copy link
Member

laanwj commented May 23, 2017

Do we need to mention the Python 3 requirement somewhere?

So the difference is that 'make check' now requires Python 3?
We could note it, thoough the same will be the case with #10044.

@jnewbery
Copy link
Contributor Author

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)

@sipa
Copy link
Member

sipa commented May 23, 2017

Concept ACK

Copy link
Contributor

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

Otherwise looks good.

Copy link
Contributor

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.

@jnewbery
Copy link
Contributor Author

Thanks @TheBlueMatt . Block restored in fixup commit. I'll squash when this is ready for merge.

@maflcko
Copy link
Member

maflcko commented Jun 6, 2017

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.
@jnewbery jnewbery force-pushed the shared_util_function_test_config branch from 23174d3 to 8ad5bde Compare June 6, 2017 20:53
@jnewbery
Copy link
Contributor Author

jnewbery commented Jun 6, 2017

fixup commits squashed

@maflcko
Copy link
Member

maflcko commented Jun 6, 2017

utACK 8ad5bde

@maflcko maflcko merged commit 8ad5bde into bitcoin:master Jun 6, 2017
maflcko pushed a commit that referenced this pull request Jun 6, 2017
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
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 8, 2019
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
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants