[tests] Allow tests to pass when stderr is non-empty #10703

pull jnewbery wants to merge 1 commits into bitcoin:master from jnewbery:test_stderr changing 1 files +34 −17

jnewbery commented at 1:35 PM on June 29, 2017: member

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.

[tests] Allow tests to pass when stderr is non-empty

Tests which pass with non-empty stderr are reported as "passed with
warnings"

d64ac3f4aa

jonasschnelli added the label Tests on Jun 29, 2017

in test/functional/test_runner.py:43 in d64ac3f4aa

  38 | @@ -40,17 +39,28 @@
  39 |      CROSS = "x "
  40 |      CIRCLE = "o "
  41 |  
  42 | +# Default colors to empty strings.
  43 | +BOLD, BLUE, RED, GREY, MAGENTA = [("", "")] * 5

jimmysong commented at 6:45 PM on June 30, 2017:

nit: alpha sort other than BOLD?

in test/functional/test_runner.py:51 in d64ac3f4aa

  48 | -    BLUE = ('\033[0m', '\033[0;34m')
  49 | -    RED = ('\033[0m', '\033[0;31m')
  50 |      GREY = ('\033[0m', '\033[1;30m')
  51 | +    RED = ('\033[0m', '\033[0;31m')
  52 | +    BLUE = ('\033[0m', '\033[0;34m')
  53 | +    MAGENTA = ('\033[0m', '\033[0;35m')

jimmysong commented at 6:46 PM on June 30, 2017:

nit: alpha sort?

in test/functional/test_runner.py:60 in d64ac3f4aa

  57 |  
  58 | +STATUS_PASSED = "Passed"
  59 | +STATUS_PASSED_WITH_WARNINGS = "Passed with warnings"
  60 | +STATUS_SKIPPED = "Skipped"
  61 | +STATUS_FAILED = "Failed"
  62 | +STATUSES = [STATUS_PASSED, STATUS_PASSED_WITH_WARNINGS, STATUS_SKIPPED, STATUS_FAILED]

jimmysong commented at 6:48 PM on June 30, 2017:

This doesn't seem to be used anywhere else, maybe combine with line below?

meshcollider commented at 8:20 AM on July 1, 2017:

IMO its cleaner how it is, shouldn't be combined

jimmysong commented at 6:48 PM on June 30, 2017: contributor

utACK, a few nits.

in test/functional/test_runner.py:444 in d64ac3f4aa

 444 | -            color = RED
 445 | -            glyph = CROSS
 446 | -        elif self.status == "Skipped":
 447 | +        if self.status == STATUS_PASSED_WITH_WARNINGS:
 448 | +            color = MAGENTA
 449 | +            glyph = TICK

meshcollider commented at 8:41 AM on July 1, 2017:

Is TICK the best glyph to use for passed with warnings? Or something more neutral?

meshcollider commented at 8:41 AM on July 1, 2017: contributor

utACK https://github.com/bitcoin/bitcoin/pull/10703/commits/d64ac3f4aab13a1a4e8d320b52e7a2b934e011b2
sipa commented at 10:53 AM on July 25, 2017: member

The code here looks like mostly output formatting changes, not actually dealing with the issue described by the title?

in test/functional/test_runner.py:62 in d64ac3f4aa

  59 | +STATUS_PASSED_WITH_WARNINGS = "Passed with warnings"
  60 | +STATUS_SKIPPED = "Skipped"
  61 | +STATUS_FAILED = "Failed"
  62 | +STATUSES = [STATUS_PASSED, STATUS_PASSED_WITH_WARNINGS, STATUS_SKIPPED, STATUS_FAILED]
  63 | +
  64 | +STATUS_MAX_LEN = max([len(st) for st in STATUSES])

ryanofsky commented at 6:48 PM on July 25, 2017:

Square brackets unneeded and probably should be dropped for slightly better readability & efficiency.

ryanofsky commented at 6:59 PM on July 25, 2017: member

utACK d64ac3f4aab13a1a4e8d320b52e7a2b934e011b2. Note that #10882 is currently depending on this so it needs 0.15 milestone.

The code here looks like mostly output formatting changes, not actually dealing with the issue described by the title?

"Allow tests to pass" implies output formatting has to change at least somewhat to avoid giving impression that tests failed when they actually succeeded, and these changes are a pretty straightforward way of updating the output.
MarcoFalke commented at 7:19 PM on July 25, 2017: member

utACK d64ac3f4aab13a1a4e8d320b52e7a2b934e011b2
MarcoFalke merged this on Jul 25, 2017
MarcoFalke closed this on Jul 25, 2017
MarcoFalke referenced this in commit f1f1605c22 on Jul 25, 2017
laanwj commented at 7:48 PM on July 25, 2017: member

Reverted this in 60f9778 after discussion on IRC. Sorry @MarcoFalke but there seems no agreement on doing this change. It's unfortunate that #10882 requires this, but it seems like we'll have to look for an alternative solution there.
jnewbery deleted the branch on Jul 26, 2017
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#10241 Allow tests to pass even when stderr got populated