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

1. 
   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.

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

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

   d64ac3f4aa
3. jonasschnelli added the label Tests on Jun 29, 2017
4. 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?
5. 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?
6. 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
7. 
   jimmysong commented at 6:48 pm on June 30, 2017: contributor

   utACK, a few nits.
8. 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?
9. 
   meshcollider commented at 8:41 am on July 1, 2017: contributor

   utACK https://github.com/bitcoin/bitcoin/pull/10703/commits/d64ac3f4aab13a1a4e8d320b52e7a2b934e011b2
10. 
    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?
11. 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.
12. 
    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.

13. 
    MarcoFalke commented at 7:19 pm on July 25, 2017: member

    utACK d64ac3f4aab13a1a4e8d320b52e7a2b934e011b2
14. MarcoFalke merged this on Jul 25, 2017
15. MarcoFalke closed this on Jul 25, 2017

    ---

16. MarcoFalke referenced this in commit f1f1605c22 on Jul 25, 2017
17. 
    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.
18. jnewbery deleted the branch on Jul 26, 2017
19. MarcoFalke locked this on Sep 8, 2021

Contributors
jnewbery jimmysong meshcollider sipa ryanofsky MarcoFalke laanwj

Labels
Tests