Skip to content
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

[Tests] Require exact match in assert_start_raises_init_eror (jnewbery) #12718

Merged
merged 3 commits into from
Mar 22, 2018

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Mar 18, 2018

Extracted from #12379, because the changes are important on their own.

This allows for exact testing, since the match can be specified with a strict regex. Internal details (such as exact formatting of the error message) can still be fuzzed away by regex wildcards.

@maflcko maflcko added the Tests label Mar 18, 2018
@maflcko maflcko force-pushed the Mf1803-qaRegexInitError branch 2 times, most recently from fac49c1 to 5812273 Compare March 18, 2018 18:41
@laanwj
Copy link
Member

laanwj commented Mar 19, 2018

I did a similar thing in #12302 initially and no one agreed :/

@maflcko
Copy link
Member Author

maflcko commented Mar 19, 2018

Hmm, I didn't notice the initial version of #12302, otherwise it would have been a Concept ACK from me

@laanwj
Copy link
Member

laanwj commented Mar 19, 2018

Yeah doesn't matter, utACK

@maflcko maflcko requested a review from jnewbery March 19, 2018 17:14
@jnewbery
Copy link
Contributor

Tested ACK 5812273

Should be tested on other platforms in case stderr is different. See #12379 (comment) for example

I did a similar thing in #12302 initially and no one agreed :/

I think everyone agreed, but I argued that we should do the minimal fix required to get the v0.16 rc building and passing the tests.

@maflcko
Copy link
Member Author

maflcko commented Mar 19, 2018

@Sjors Do they still fail for you?

raise AssertionError('Expected message "{}" does not match stderr:\n"{}"'.format(expected_msg, stderr))
if partial_match and re.search(expected_msg, stderr) is None:
raise AssertionError('Expected message "{}" does not parially match stderr:\n"{}"'.format(expected_msg, stderr))
elif re.fullmatch(expected_msg + '\n', stderr) is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is wrong. Should be elif not partial_match and re.fullmatch(...)

or nest the whole thing one level deeper:

if partial match:
    if re.search(...) is None:
        #assert
else:
    if re.fullmatch(...) is None:
        #assert

This allows the tests to pass on different platforms
@maflcko maflcko force-pushed the Mf1803-qaRegexInitError branch from fad4973 to fae1374 Compare March 19, 2018 19:49
@maflcko
Copy link
Member Author

maflcko commented Mar 19, 2018

Added a commit to make the tests pass on windows

@jnewbery
Copy link
Contributor

Tested ACK fae1374. Works for me.

@Sjors
Copy link
Member

Sjors commented Mar 21, 2018

@MarcoFalke on MacOS 10.13.3. wallet_encryption.py still fails with a timeout. wallet_multiwallet.py and wallet_multiwallet.py --usecli now pass.

Running the full suite now; will update this comment if something new fails.

@laanwj
Copy link
Member

laanwj commented Mar 22, 2018

Tested ACK fae1374

@MarcoFalke on MacOS 10.13.3. wallet_encryption.py still fails with a timeout.

Huh - we don't use any remotely complex regexes, so I wonder how this can cause timeout.

@laanwj laanwj merged commit fae1374 into bitcoin:master Mar 22, 2018
laanwj added a commit that referenced this pull request Mar 22, 2018
…_eror (jnewbery)

fae1374 qa: Allow for partial_match when checking init error (MarcoFalke)
5812273 [Tests] Require exact match in assert_start_raises_init_eror() (John Newbery)
0ec08a6 [Tests] Move assert_start_raises_init_error method to TestNode (John Newbery)

Pull request description:

  Extracted from #12379, because the changes are important on their own.

  This allows for exact testing, since the match can be specified with a strict regex. Internal details (such as exact formatting of the error message) can still be fuzzed away by regex wildcards.

Tree-SHA512: 605d2c9c42362a32d42321b066637577a026d0bb8cfc1c9f5737a4ca6503ffe85457a5122cea6e1101053ccc6c8aa1bbae3602e1fa7d2988bf7d5c275f412f66
@maflcko maflcko deleted the Mf1803-qaRegexInitError branch March 22, 2018 09:51
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Sep 27, 2020
…es_init_eror (jnewbery)

fae1374 qa: Allow for partial_match when checking init error (MarcoFalke)
5812273 [Tests] Require exact match in assert_start_raises_init_eror() (John Newbery)
0ec08a6 [Tests] Move assert_start_raises_init_error method to TestNode (John Newbery)

Pull request description:

  Extracted from bitcoin#12379, because the changes are important on their own.

  This allows for exact testing, since the match can be specified with a strict regex. Internal details (such as exact formatting of the error message) can still be fuzzed away by regex wildcards.

Tree-SHA512: 605d2c9c42362a32d42321b066637577a026d0bb8cfc1c9f5737a4ca6503ffe85457a5122cea6e1101053ccc6c8aa1bbae3602e1fa7d2988bf7d5c275f412f66
Signed-off-by: pasta <[email protected]>
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Oct 22, 2020
…es_init_eror (jnewbery)

fae1374 qa: Allow for partial_match when checking init error (MarcoFalke)
5812273 [Tests] Require exact match in assert_start_raises_init_eror() (John Newbery)
0ec08a6 [Tests] Move assert_start_raises_init_error method to TestNode (John Newbery)

Pull request description:

  Extracted from bitcoin#12379, because the changes are important on their own.

  This allows for exact testing, since the match can be specified with a strict regex. Internal details (such as exact formatting of the error message) can still be fuzzed away by regex wildcards.

Tree-SHA512: 605d2c9c42362a32d42321b066637577a026d0bb8cfc1c9f5737a4ca6503ffe85457a5122cea6e1101053ccc6c8aa1bbae3602e1fa7d2988bf7d5c275f412f66
Signed-off-by: pasta <[email protected]>
random-zebra added a commit to PIVX-Project/PIVX that referenced this pull request Aug 1, 2021
84768b5 scripted-diff: bitcoind-->pivxd in tests (random-zebra)
38dae98 qa: Match full plain text by default (MarcoFalke)
92977e5 [Tests] Fix invalid escapes in regex strings to fix W605 (random-zebra)
b6d69ea qa: Allow for partial_match when checking init error (MarcoFalke)
0318a5b [Trivial] Remove extra newline in missing-blocksdir error (random-zebra)
496787a [Tests] Require exact match in assert_start_raises_init_eror() (John Newbery)
6617a99 [Tests] Move assert_start_raises_init_error method to TestNode (random-zebra)

Pull request description:

  Backports
  - bitcoin#12718

  This completes the todo left for
  - #2449.

ACKs for top commit:
  furszy:
    no other changes aside from the typo, utACK 84768b5
  Fuzzbawls:
    ACK 84768b5

Tree-SHA512: 50d527c965bcaf9f07c690cbbeac6c650e88055b1ac4a6f7920e11f3e0f480a8b059e5bd5b4023d7bcc5f1638677aac3c7faf243631d63ce94dca6ad2dd60797
random-zebra added a commit to PIVX-Project/PIVX that referenced this pull request Aug 5, 2021
63e0be6 [Remove] By-pass logprint-scanner restriction. (furszy)
280ced3 utils: Fix broken Windows filelock (Chun Kuan Lee)
be89860 utils: Convert Windows args to utf-8 string (Chun Kuan Lee)
e8cfa6e Call unicode API on Windows (Chun Kuan Lee)
1a02a8a tests: Add test case for std::ios_base::ate (Chun Kuan Lee)
2e57cd4 Move boost/std fstream to fsbridge (furszy)
9d8bcd4 utils: Add fsbridge fstream function wrapper (Chun Kuan Lee)
d59d48d utils: Convert fs error messages from multibyte to utf-8 (ken2812221)
9ef58cc Logging: use "fmterr" variable name for errors instead of general "e" that can be used by any other function. (furszy)
dd94241 utils: Use _wfopen and _wreopen on Windows (Chun Kuan Lee)
3993641 add unicode compatible file_lock for Windows (Chun Kuan Lee)
48349f8 Provide relevant error message if datadir is not writable. (murrayn)

Pull request description:

  As the software is currently using the ANSI encoding on Windows, the user's language settings could affect the proper functioning of the node/wallet, to the point of not be able to open some non-ASCII name files and directories.

  This solves the Windows encoding issues, completing the entire bitcoin#13869 work path (and some other required backports). Enabling for example users that use non-english characters in directories and file names to be accepted.

  Backported PRs:
  * bitcoin#12422.
  * bitcoin#12630.
  * bitcoin#13862.
  * bitcoin#13866.
  * bitcoin#13877.
  * bitcoin#13878.
  * bitcoin#13883.
  * bitcoin#13884.
  * bitcoin#13886.
  * bitcoin#13888.
  * bitcoin#14192.
  * bitcoin#13734.
  * bitcoin#14426.

  This is built on top of other two PRs that i have open #2423 and #2369.
  Solves old issues #940 and #2163.

  TODO:
  * Backport `assert_start_raises_init_error` and `ErrorMatch` in TestNode` (bitcoin#12718)

ACKs for top commit:
  Fuzzbawls:
    ACK 63e0be6
  random-zebra:
    ACK 63e0be6 and merging...

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

4 participants