Skip to content

test: Make ua_comment test pass on 0.16.0 #12302

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.

Sign up for GitHub

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

Jump to bottom
Merged
maflcko merged 1 commit into from
Jan 30, 2018
Merged

test: Make ua_comment test pass on 0.16.0 #12302

maflcko merged 1 commit into from
Jan 30, 2018

Conversation

@laanwj
Copy link
Member

@laanwj laanwj commented Jan 30, 2018

The specific length of the uacomment is one shorter on 0.16.0 than on 0.15.99 causing the (stupid) test to fail.

This change makes assert_start_raises_init_error optionally take a regexp, so that the error message can be checked without being specific about the reported length.

@laanwj laanwj added the Tests label Jan 30, 2018
@laanwj laanwj added this to the 0.16.0 milestone Jan 30, 2018
@theuni
Copy link
Member

theuni commented Jan 30, 2018

utACK 0224be7bb45f0faed13ba067d14082f19cc0685a

promag
promag reviewed Jan 30, 2018
View reviewed changes
Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

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

utACK.

test/functional/feature_uacomment.py Outdated
Copy link
Contributor

@promag promag Jan 30, 2018

Choose a reason for hiding this comment

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

nit, \d+

test/functional/test_framework/test_framework.py Outdated
Copy link
Contributor

@promag promag Jan 30, 2018

Choose a reason for hiding this comment

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

nit nit, always treat as regexp and remove option?

Copy link
Member Author

@laanwj laanwj Jan 30, 2018

Choose a reason for hiding this comment

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

no, then I'd have to change all the tests

@laanwj laanwj force-pushed the 2017_01_uacomment_test_fix branch from 0224be7 to 031c9a0 Compare January 30, 2018 18:28
@jnewbery
Copy link
Contributor

jnewbery commented Jan 30, 2018 •
edited
Loading

Why not just the minimal fix of:

--- a/test/functional/feature_uacomment.py
+++ b/test/functional/feature_uacomment.py
@@ -23,7 +23,7 @@ class UacommentTest(BitcoinTestFramework):
 
         self.log.info("test -uacomment max length")
         self.stop_node(0)
-        expected = "Total length of network version string (286) exceeds maximum length (256). Reduce the number or size of uacomments."
+        expected = "exceeds maximum length (256). Reduce the number or size of uacomments."
         self.assert_start_raises_init_error(0, ["-uacomment=" + 'a' * 256], expected)

Rather than changing the assert_start_raises_init_error() util function for this one case.

@laanwj
Copy link
Member Author

laanwj commented Jan 30, 2018

Sigh...

@laanwj laanwj closed this Jan 30, 2018
@laanwj
Copy link
Member Author

laanwj commented Jan 30, 2018

Closing this, feel free to fix it in your own way.

@sdaftuar
Copy link
Member

sdaftuar commented Jan 30, 2018

ACK 0224be7bb45f0faed13ba067d14082f19cc0685a (verified that it fixes 0.16.0)
utACK 031c9a0

Let's just merge a fix and move on, this is too much bikeshedding imo.

@laanwj laanwj reopened this Jan 30, 2018
@laanwj laanwj force-pushed the 2017_01_uacomment_test_fix branch from 031c9a0 to 197982b Compare January 30, 2018 20:19
@laanwj
test: Make ua_comment test pass on 0.16.0
aac6bce 
The specific length of the uacomment is one shorter on `0.16.0` than on
`0.15.99` causing the (stupid) test to fail.
Just match the latter part of the message only.
@laanwj laanwj force-pushed the 2017_01_uacomment_test_fix branch from 197982b to aac6bce Compare January 30, 2018 20:20
@jnewbery
Copy link
Contributor

jnewbery commented Jan 30, 2018

ACK aac6bce

I agree with the longer term goal of changing assert_start_raises_init_error() to compare regexs. There are lots of other tests that check substrings which could be improved.

@maflcko
Copy link
Member

maflcko commented Jan 30, 2018

utACK aac6bce

Didn't test that this fixes 0.16. Going to merge and cross fingers...

@maflcko maflcko merged commit aac6bce into bitcoin:master Jan 30, 2018
maflcko pushed a commit that referenced this pull request Jan 30, 2018
Merge #12302: test: Make ua_comment test pass on 0.16.0
f0295be 
aac6bce test: Make ua_comment test pass on 0.16.0 (Wladimir J. van der Laan)

Pull request description:

  The specific length of the uacomment is one shorter on `0.16.0` than on `0.15.99` causing the (stupid) test to fail.

  This change makes `assert_start_raises_init_error` optionally take a regexp, so that the error message can be checked without being specific about the reported length.

Tree-SHA512: 1c5e9f1d0b36f004f8113c7e211e8281194ce8057869ac3702172d8b021df3c3aa3b8f79b9434678ed0fb7146f848802410647d434fc884aa200b6a5e26afe8b
@jnewbery
Copy link
Contributor

jnewbery commented Jan 30, 2018

(also tested it fixes v0.16 for me)

@bitcoin bitcoin deleted a comment Jan 30, 2018
jnewbery pushed a commit to jnewbery/bitcoin that referenced this pull request Jan 30, 2018
@laanwj @jnewbery
test: Make ua_comment test pass on 0.16.0
c4134f8 
The specific length of the uacomment is one shorter on `0.16.0` than on
`0.15.99` causing the (stupid) test to fail.
Just match the latter part of the message only.

Github-Pull: bitcoin#12302
Rebased-From: aac6bce
@jnewbery jnewbery mentioned this pull request Jan 30, 2018
Closed
laanwj added a commit that referenced this pull request Jan 30, 2018
@laanwj
test: Make ua_comment test pass on 0.16.0
6c2788c 
The specific length of the uacomment is one shorter on `0.16.0` than on
`0.15.99` causing the (stupid) test to fail.
Just match the latter part of the message only.

Github-Pull: #12302
Rebased-From: aac6bce
Tree-SHA512: 9edecbe2529584d6d01296ec153330bb44add8445fef139d7b7a667b86fef8ee3aafea55d95ac109c9fef079133709f69798477e3eba92744ea2f6c8f5acbb7d
@jnewbery jnewbery mentioned this pull request Feb 7, 2018
Closed
HashUnlimited pushed a commit to chaincoin/chaincoin that referenced this pull request Mar 16, 2018
@laanwj
test: Make ua_comment test pass on 0.16.0
cb0b197 
The specific length of the uacomment is one shorter on `0.16.0` than on
`0.15.99` causing the (stupid) test to fail.
Just match the latter part of the message only.

Github-Pull: bitcoin#12302
Rebased-From: aac6bce
Tree-SHA512: 9edecbe2529584d6d01296ec153330bb44add8445fef139d7b7a667b86fef8ee3aafea55d95ac109c9fef079133709f69798477e3eba92744ea2f6c8f5acbb7d
@laanwj laanwj mentioned this pull request Mar 19, 2018
Merged
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Mar 15, 2020
@PastaPastaPasta
Merge bitcoin#12302: test: Make ua_comment test pass on 0.16.0
1948435 
aac6bce test: Make ua_comment test pass on 0.16.0 (Wladimir J. van der Laan)

Pull request description:

  The specific length of the uacomment is one shorter on `0.16.0` than on `0.15.99` causing the (stupid) test to fail.

  This change makes `assert_start_raises_init_error` optionally take a regexp, so that the error message can be checked without being specific about the reported length.

Tree-SHA512: 1c5e9f1d0b36f004f8113c7e211e8281194ce8057869ac3702172d8b021df3c3aa3b8f79b9434678ed0fb7146f848802410647d434fc884aa200b6a5e26afe8b
@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.

Reviewers

1 more reviewer

@promag promag promag left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Tests

Projects

None yet

Milestone

0.16.0

Development

Successfully merging this pull request may close these issues.

6 participants

@laanwj @theuni @jnewbery @sdaftuar @maflcko @promag