Skip to content

Conversation

@jonasschnelli
Copy link
Contributor

No description provided.

@laanwj
Copy link
Member

laanwj commented Apr 21, 2017

I thought about this a bit: apparently some people use the stderr output to determine whether a test emitted e.g. sanitizer warnings.
Possible idea: add a "WARNINGS" status, that is interpreted as PASS for the final tally but does show that something unexpected was printed.

@jonasschnelli
Copy link
Contributor Author

Added a new status "Passed with warnings" which gets set when the test passed but stderr was not empty. The test will be flagged as "successful" for that new status.

color = BLUE
glyph = TICK
if self.status == "Passed with warnings":
color = ORANGE
Copy link
Member

Choose a reason for hiding this comment

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

NameError: name 'ORANGE' is not defined

Copy link
Contributor

@jnewbery jnewbery left a comment

Choose a reason for hiding this comment

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

Couple of comments inline.

If you add a new status string, you'll need to update print justification (otherwise I think your new category will just be truncated). Justification is on L337 in print_results() and L422 in TestResult.__repr__().

Now that we have 4 categories, it may make sense to define all the categories as constants at the top of the file and have a new MAX_STATUS_WIDTH constant as well.

One you've done that, can you add a screenshot of what this looks like?

color = BLUE
glyph = TICK
if self.status == "Passed with warnings":
color = ORANGE
Copy link
Contributor

Choose a reason for hiding this comment

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

ORANGE would need to be a constant defined at the top of this file (see around L47)

if self.status == "Passed":
color = BLUE
glyph = TICK
if self.status == "Passed with warnings":
Copy link
Contributor

Choose a reason for hiding this comment

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

elif please

@maflcko
Copy link
Member

maflcko commented May 14, 2017

Concept ACK, but I will close this pull in case no one picks it up in the next couple of days.

@maflcko
Copy link
Member

maflcko commented May 18, 2017

Closing for now. Feel free to pick this up again.

@maflcko maflcko closed this May 18, 2017
maflcko pushed a commit that referenced this pull request Jul 25, 2017
d64ac3f [tests] Allow tests to pass when stderr is non-empty (Jonas Schnelli)

Pull request description:

  Resurrect #10241 with nits addressed

  Not sure how much people want this. Would be useful for functional tests which cause bitcoind to print to stderr.

Tree-SHA512: 28caccf7818fb3ed5a38caef7f77161b1678aa9b8fd12c2d1e76032f409f0d33c40f7ac91e0c8d908df4a44fd01cf97d657a08bae50c6ff17d07f5b2e20c3a6e
@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