Bugfix & simplify bn2vch using int.to_bytes#18378
Merged
laanwj merged 2 commits intobitcoin:masterfrom Mar 19, 2020
Merged
Conversation
maflcko
reviewed
Mar 18, 2020
sipa
force-pushed
the
202003_simple_bn2vch
branch
2 times, most recently
from
c3e08bf to
57d1428
Compare
sipa
force-pushed
the
202003_simple_bn2vch
branch
from
57d1428 to
a733ad5
Compare
Contributor
|
Concept ACK. The code was broken here: f950ec2#diff-fb7bd6a727cfb726928c6f68fa8b3a80R20. |
jnewbery
approved these changes
Mar 18, 2020
Contributor
jnewbery
left a comment
jnewbery
left a comment
There was a problem hiding this comment.
Tested ACK a733ad5.
I cherry-picked just the test onto:
Longer term, I think it'd be nicer if the tests for the test framework were unit tests within those files, because it doesn't make much sense to me to have them as subclasses of the TestFramework object, and because it's nice to have the test code next to the code it's testing. However, that's something that could be done separately.
Contributor
|
Concept ACK |
Member
Author
|
@jnewbery Yeah, that's pretty ugly. Happy to take any concrete suggestions (in this PR or later). |
Contributor
Let's do it in a follow-up. |
Member
|
nice, ACK a733ad5 |
maflcko
pushed a commit
that referenced
this pull request
Apr 30, 2020
de8905a test: use unittest and test_runner for test framework unit testing (Gloria Zhao) Pull request description: Proposal for unit testing on test_framework functions: 1. Use the python `unittest` library. Don't use test_framework to test itself. 2. Put the tests inside the same file as the functions they are testing. 3. Call the tests from `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](#18378 (review))). ACKs for top commit: jnewbery: Tested ACK de8905a. Great stuff gzhao408 . Thanks for this! Tree-SHA512: 91572d43e203a1864765b93a9472667994115ec38b271f2b2f9fcd0f0112b393fc24ba7d2371d5a34b0a6a4522f6b934fc5164363819aa7ed8d6c6c9a60cc101
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
May 2, 2020
de8905a test: use unittest and test_runner for test framework unit testing (Gloria Zhao) Pull request description: Proposal for unit testing on test_framework functions: 1. Use the python `unittest` library. Don't use test_framework to test itself. 2. Put the tests inside the same file as the functions they are testing. 3. Call the tests from `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](bitcoin#18378 (review))). ACKs for top commit: jnewbery: Tested ACK de8905a. Great stuff gzhao408 . Thanks for this! Tree-SHA512: 91572d43e203a1864765b93a9472667994115ec38b271f2b2f9fcd0f0112b393fc24ba7d2371d5a34b0a6a4522f6b934fc5164363819aa7ed8d6c6c9a60cc101
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Jan 9, 2021
Summary: Add a new test for a helper function in the test framework (added in D8852) Introduce a new prefix for functional tests: `framework_test` This concludes backport of Core [[bitcoin/bitcoin#18378 | PR18378]] bitcoin/bitcoin@a733ad5 Depends on D8852 Test Plan: `test/functional/test_runner.py framework_test_script` Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Differential Revision: https://reviews.bitcoinabc.org/D8855
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Alternative to #18374, fixing the incorrect padding added sometimes in
bn2vch.Since we're using Python 3.2+, a much simpler implementation of
bn2vchis possible usingint.to_bytes.This also adds a "functional" test for bn2vch, in a new "framework_test_script.py", where the "framework_test_" prefix is intended for tests of the framework itself.