bpo-34945: Buffer output in test suite only when creating junit file #10204

pablogsal · 2018-10-28T22:56:08Z

After commit d0f49d2, the output of the
test suite is always buffered as the test output needs to be included in
the JUnit file in same cases (as when a test fails). This has the
consequence that printing or using debuggers (like pdb) in the test
suite does not result in a good user experience anymore.

This commit modifies the test suite runner so it only captures the test
output when the JUnit file is requested to fix the regression so prints
and debuggers are usable again.

CC: @matrixise

https://bugs.python.org/issue34945

vstinner · 2018-10-28T23:20:14Z

Lib/test/support/testresult.py

-            ET.SubElement(e, 'system-out').text = stdout
-            stderr = self._stderr_buffer.getvalue().rstrip()
-            ET.SubElement(e, 'system-err').text = stderr
+            if self._stderr_buffer:


Ups! I forgot to add that file when pushing the latest fix

After commit d0f49d2, the output of the test suite is always buffered as the test output needs to be included in the JUnit file in same cases (as when a test fails). This has the consequence that printing or using debuggers (like pdb) in the test suite does not result in a good user experience anymore. This commit modifies the test suite runner so it only captures the test output when the JUnit file is requested to fix the regression so prints and debuggers are usable again.

vstinner

It doesn't work for me: I don't see my print(). It works before the JUnit change.

vstinner@apu$ ./python -m test test_os
Run tests sequentially
0:00:00 load avg: 0.34 [1/1] test_os
git dif
== Tests result: SUCCESS ==

1 test OK.

Total duration: 1 sec 436 ms
Tests result: SUCCESS
vstinner@apu$ git diff
diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py
index 3f6e48f0c8..716b079a21 100644
--- a/Lib/test/test_os.py
+++ b/Lib/test/test_os.py
@@ -99,6 +99,7 @@ class FileTests(unittest.TestCase):
 
     def test_access(self):
         f = os.open(support.TESTFN, os.O_CREAT|os.O_RDWR)
+        print("test_access")
         os.close(f)
         self.assertTrue(os.access(support.TESTFN, os.W_OK))

bedevere-bot · 2018-10-29T13:58:56Z

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

pablogsal · 2018-10-29T14:23:17Z

@vstinner Can your run it in verbose mode?

./python -m test test_os -v

pablogsal · 2018-10-29T14:26:42Z

This is because the JUnit change made this choice:

cpython/Lib/test/support/testresult.py

Line 170 in 8f094a7

return QuietRegressionTestRunner

vstinner · 2018-10-29T14:28:04Z

@vstinner Can your run it in verbose mode?

I would like to get back the old behavior: see my print() without -v.
https://bugs.python.org/issue34945#msg327462

The commit d0f49d2 changed the behavior and it wasn't the intent of Steve Dower.
https://bugs.python.org/issue34945#msg327658

pablogsal · 2018-10-29T16:35:04Z

I have made the requested changes; please review again

bedevere-bot · 2018-10-29T16:35:11Z

Thanks for making the requested changes!

@zooba, @vstinner: please review the changes made to this pull request.

vstinner

LGTM. The functional test now works as expected: adding print("XXX") now displays XXX in stdout ;-)

Just a minor suggestion.

vstinner · 2018-10-29T18:15:35Z

Lib/test/support/testresult.py

-            ET.SubElement(e, 'system-out').text = stdout
-            stderr = self._stderr_buffer.getvalue().rstrip()
-            ET.SubElement(e, 'system-err').text = stderr
+            if self._stdout_buffer:


Maybe use "is not None"?

vstinner · 2018-10-29T20:31:40Z

Since JUnit has been added to other branches, this change should be ported to other branches, no?

miss-islington · 2018-10-29T20:46:29Z

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6.
🐍🍒⛏🤖

miss-islington · 2018-10-29T20:46:29Z

Thanks @pablogsal for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

…ythonGH-10204) After commit d0f49d2, the output of the test suite is always buffered as the test output needs to be included in the JUnit file in same cases (as when a test fails). This has the consequence that printing or using debuggers (like pdb) in the test suite does not result in a good user experience anymore. This commit modifies the test suite runner so it only captures the test output when the JUnit file is requested to fix the regression so prints and debuggers are usable again. (cherry picked from commit 0227748) Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>

bedevere-bot · 2018-10-29T20:46:40Z

GH-10227 is a backport of this pull request to the 3.7 branch.

…ythonGH-10204) After commit d0f49d2, the output of the test suite is always buffered as the test output needs to be included in the JUnit file in same cases (as when a test fails). This has the consequence that printing or using debuggers (like pdb) in the test suite does not result in a good user experience anymore. This commit modifies the test suite runner so it only captures the test output when the JUnit file is requested to fix the regression so prints and debuggers are usable again. (cherry picked from commit 0227748) Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>

bedevere-bot · 2018-10-29T20:46:49Z

GH-10228 is a backport of this pull request to the 3.6 branch.

…H-10204) After commit d0f49d2, the output of the test suite is always buffered as the test output needs to be included in the JUnit file in same cases (as when a test fails). This has the consequence that printing or using debuggers (like pdb) in the test suite does not result in a good user experience anymore. This commit modifies the test suite runner so it only captures the test output when the JUnit file is requested to fix the regression so prints and debuggers are usable again. (cherry picked from commit 0227748) Co-authored-by: Pablo Galindo <Pablogsal@gmail.com>

pablogsal added the skip news label Oct 28, 2018

pablogsal self-assigned this Oct 28, 2018

pablogsal requested review from vstinner and zooba October 28, 2018 22:56

the-knights-who-say-ni added the CLA signed label Oct 28, 2018

bedevere-bot added the awaiting merge label Oct 28, 2018

pablogsal force-pushed the bpo34945 branch from a6d1856 to 4df79c7 Compare October 28, 2018 23:15

vstinner reviewed Oct 28, 2018

View reviewed changes

pablogsal force-pushed the bpo34945 branch from 4df79c7 to 8f094a7 Compare October 28, 2018 23:24

zooba approved these changes Oct 29, 2018

View reviewed changes

vstinner requested changes Oct 29, 2018

View reviewed changes

bedevere-bot removed the awaiting merge label Oct 29, 2018

bedevere-bot added the awaiting changes label Oct 29, 2018

pablogsal force-pushed the bpo34945 branch from 0735296 to 5e338be Compare October 29, 2018 15:12

Do not capture output when running without -v as well

832f673

pablogsal force-pushed the bpo34945 branch from 5e338be to 832f673 Compare October 29, 2018 15:15

bedevere-bot added awaiting change review and removed awaiting changes labels Oct 29, 2018

vstinner approved these changes Oct 29, 2018

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Oct 29, 2018

Check for identity on None

27d25d6

pablogsal merged commit 0227748 into python:master Oct 29, 2018

bedevere-bot removed the awaiting merge label Oct 29, 2018

pablogsal deleted the bpo34945 branch October 29, 2018 20:09

pablogsal added needs backport to 3.6 labels Oct 29, 2018

bedevere-bot removed the needs backport to 3.7 label Oct 29, 2018

bedevere-bot removed the needs backport to 3.6 label Oct 29, 2018

Uh oh!

bpo-34945: Buffer output in test suite only when creating junit file #10204

bpo-34945: Buffer output in test suite only when creating junit file #10204

Uh oh!

Conversation

pablogsal commented Oct 28, 2018 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner Oct 28, 2018

Choose a reason for hiding this comment

Uh oh!

pablogsal Oct 28, 2018

Choose a reason for hiding this comment

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Oct 29, 2018

Uh oh!

pablogsal commented Oct 29, 2018

Uh oh!

pablogsal commented Oct 29, 2018

Uh oh!

vstinner commented Oct 29, 2018

Uh oh!

pablogsal commented Oct 29, 2018

Uh oh!

bedevere-bot commented Oct 29, 2018

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner Oct 29, 2018

Choose a reason for hiding this comment

Uh oh!

vstinner commented Oct 29, 2018

Uh oh!

miss-islington commented Oct 29, 2018

Uh oh!

miss-islington commented Oct 29, 2018

Uh oh!

bedevere-bot commented Oct 29, 2018

Uh oh!

bedevere-bot commented Oct 29, 2018

Uh oh!

Uh oh!

pablogsal commented Oct 28, 2018 •

edited by bedevere-bot

Loading