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.

Sign up for GitHub

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

Jump to bottom

test: Fix unit test logging with python3 #31156

Closed
wants to merge 1 commit into from
Closed

test: Fix unit test logging with python3 #31156

wants to merge 1 commit into from

Conversation

AdamMajer
Copy link
Contributor

A logfile must be opened as a text file and encoding is specified.
Then it can accept a string. Current behaviour we get,

Message: 'ok 2834 sequential/test-worker-prof'
Arguments: ()
--- Logging error ---
Traceback (most recent call last):
File "/usr/lib64/python3.7/logging/init.py", line 1037, in emit
stream.write(msg + self.terminator)
TypeError: a bytes-like object is required, not 'str'
Call stack:
File "tools/test.py", line 1734, in
sys.exit(Main())
File "tools/test.py", line 1710, in Main
if RunTestCases(cases_to_run, options.progress, options.j, options.flaky_tests):
File "tools/test.py", line 933, in RunTestCases
return progress.Run(tasks)
File "tools/test.py", line 141, in Run
self.RunSingle(False, 0)
File "tools/test.py", line 202, in RunSingle
self.HasRun(output)
File "tools/test.py", line 365, in HasRun
logger.info(' ---')

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory. labels Jan 2, 2020
@BridgeAR BridgeAR requested a review from cclauss January 2, 2020 14:04
cclauss
cclauss approved these changes Jan 2, 2020
View reviewed changes
Copy link
Contributor

@cclauss cclauss left a comment

Choose a reason for hiding this comment

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

encoding will default to utf.8 if not specified and mode will default to wt if not specified but the Zen of Python does state that Explicit is better than implicit.

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28100/

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 2, 2020
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28136/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28152/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28170/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28194/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28203/

@cclauss
Copy link
Contributor

cclauss commented Jan 4, 2020

6 times the Jenkins test failed. Is this change breaking those tests?

@Trott
Copy link
Member

Trott commented Jan 4, 2020 •
edited
Loading

6 times the Jenkins test failed. Is this change breaking those tests?

Seems like it, on Windows at least:

10:02:14 > echo running 'python tools\test.py --mode=release  --flaky-tests=dontcare --run=0,4 -J -p tap --logfile test.tap default' 
10:02:14 running 'python tools\test.py --mode=release  --flaky-tests=dontcare --run=0,4 -J -p tap --logfile test.tap default'
10:02:14 
10:02:14 > python tools\test.py --mode=release  --flaky-tests=dontcare --run=0,4 -J -p tap --logfile test.tap default 
10:02:14 Traceback (most recent call last):
10:02:14   File "tools\test.py", line 1724, in <module>
10:02:14     sys.exit(Main())
10:02:14   File "tools\test.py", line 1537, in Main
10:02:14     fh = logging.FileHandler(options.logfile, encoding='utf-8', mode='wt')
10:02:14   File "C:\Python27\lib\logging\__init__.py", line 920, in __init__
10:02:14     StreamHandler.__init__(self, self._open())
10:02:14   File "C:\Python27\lib\logging\__init__.py", line 952, in _open
10:02:14     stream = codecs.open(self.baseFilename, self.mode, self.encoding)
10:02:14   File "C:\Python27\lib\codecs.py", line 898, in open
10:02:14     file = __builtin__.open(filename, mode, buffering)
10:02:14 ValueError: Invalid mode ('wtb')

@Trott Trott removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 4, 2020
cclauss
cclauss requested changes Jan 6, 2020
View reviewed changes
Copy link
Contributor

@cclauss cclauss left a comment

Choose a reason for hiding this comment

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

Must be addressed: #31156 (comment)

@AdamMajer
test: Fix unit test logging with python3
fda3766 
A logfile must be opened as a text file and encoding is specified.
Then it can accept a string. Current behaviour we get,

 Message: 'ok 2834 sequential/test-worker-prof'
 Arguments: ()
 --- Logging error ---
 Traceback (most recent call last):
   File "/usr/lib64/python3.7/logging/__init__.py", line 1037, in emit
     stream.write(msg + self.terminator)
 TypeError: a bytes-like object is required, not 'str'
 Call stack:
   File "tools/test.py", line 1734, in <module>
     sys.exit(Main())
   File "tools/test.py", line 1710, in Main
     if RunTestCases(cases_to_run, options.progress, options.j, options.flaky_tests):
   File "tools/test.py", line 933, in RunTestCases
     return progress.Run(tasks)
   File "tools/test.py", line 141, in Run
     self.RunSingle(False, 0)
   File "tools/test.py", line 202, in RunSingle
     self.HasRun(output)
   File "tools/test.py", line 365, in HasRun
     logger.info('  ---')
@AdamMajer
Copy link
Contributor Author

Rebased and changed the mode to just 'w'. It still functions correctly with python3 on Linux. Hopefully this will be enough for python on Windows.

@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/28299/

@Trott
Copy link
Member

Trott commented Jan 8, 2020

Landed in f2a089a

@Trott Trott closed this Jan 8, 2020
Trott pushed a commit that referenced this pull request Jan 8, 2020
@AdamMajer @Trott
test: fix unit test logging with python3
f2a089a 
A logfile must be opened as a text file and encoding is specified.
Then it can accept a string. Current behaviour we get,

 Message: 'ok 2834 sequential/test-worker-prof'
 Arguments: ()
 --- Logging error ---
 Traceback (most recent call last):
   File "/usr/lib64/python3.7/logging/__init__.py", line 1037, in emit
     stream.write(msg + self.terminator)
 TypeError: a bytes-like object is required, not 'str'
 Call stack:
   File "tools/test.py", line 1734, in <module>
     sys.exit(Main())
   File "tools/test.py", line 1710, in Main
     if RunTestCases(cases_to_run, options.progress, options.j, options.flaky_tests):
   File "tools/test.py", line 933, in RunTestCases
     return progress.Run(tasks)
   File "tools/test.py", line 141, in Run
     self.RunSingle(False, 0)
   File "tools/test.py", line 202, in RunSingle
     self.HasRun(output)
   File "tools/test.py", line 365, in HasRun
     logger.info('  ---')

PR-URL: #31156
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 16, 2020
@AdamMajer @MylesBorins
test: fix unit test logging with python3
f93095d 
A logfile must be opened as a text file and encoding is specified.
Then it can accept a string. Current behaviour we get,

 Message: 'ok 2834 sequential/test-worker-prof'
 Arguments: ()
 --- Logging error ---
 Traceback (most recent call last):
   File "/usr/lib64/python3.7/logging/__init__.py", line 1037, in emit
     stream.write(msg + self.terminator)
 TypeError: a bytes-like object is required, not 'str'
 Call stack:
   File "tools/test.py", line 1734, in <module>
     sys.exit(Main())
   File "tools/test.py", line 1710, in Main
     if RunTestCases(cases_to_run, options.progress, options.j, options.flaky_tests):
   File "tools/test.py", line 933, in RunTestCases
     return progress.Run(tasks)
   File "tools/test.py", line 141, in Run
     self.RunSingle(False, 0)
   File "tools/test.py", line 202, in RunSingle
     self.HasRun(output)
   File "tools/test.py", line 365, in HasRun
     logger.info('  ---')

PR-URL: #31156
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
@codebytere codebytere mentioned this pull request Jan 16, 2020
Merged
codebytere pushed a commit that referenced this pull request Mar 14, 2020
@AdamMajer @codebytere
test: fix unit test logging with python3
04e2f41 
A logfile must be opened as a text file and encoding is specified.
Then it can accept a string. Current behaviour we get,

 Message: 'ok 2834 sequential/test-worker-prof'
 Arguments: ()
 --- Logging error ---
 Traceback (most recent call last):
   File "/usr/lib64/python3.7/logging/__init__.py", line 1037, in emit
     stream.write(msg + self.terminator)
 TypeError: a bytes-like object is required, not 'str'
 Call stack:
   File "tools/test.py", line 1734, in <module>
     sys.exit(Main())
   File "tools/test.py", line 1710, in Main
     if RunTestCases(cases_to_run, options.progress, options.j, options.flaky_tests):
   File "tools/test.py", line 933, in RunTestCases
     return progress.Run(tasks)
   File "tools/test.py", line 141, in Run
     self.RunSingle(False, 0)
   File "tools/test.py", line 202, in RunSingle
     self.HasRun(output)
   File "tools/test.py", line 365, in HasRun
     logger.info('  ---')

PR-URL: #31156
Reviewed-By: Christian Clauss <cclauss@me.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
@codebytere codebytere mentioned this pull request Mar 17, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BridgeAR BridgeAR BridgeAR approved these changes

@richardlau richardlau richardlau approved these changes

@devnexen devnexen devnexen approved these changes

@cclauss cclauss cclauss approved these changes

@cjihrig cjihrig cjihrig approved these changes

@watilde watilde watilde approved these changes

@Trott Trott Trott approved these changes

Assignees
No one assigned
Labels
test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@AdamMajer @nodejs-github-bot @cclauss @Trott @watilde @cjihrig @devnexen @richardlau @BridgeAR