[mozlog] Move the stdio capture from web-platform-tests to mozlog.structured
Categories
(Testing :: Mozbase, defect)
Tracking
(firefox67 fixed)
| Tracking | Status | |
|---|---|---|
| firefox67 | --- | fixed |
People
(Reporter: jgraham, Assigned: nikkis)
Details
Attachments
(1 file)
The ability to capture the parent process' stdio seems like a useful feature to move out into mozlog.strucured.
|
Reporter | |
Updated•5 years ago
|
|
Assignee | |
Comment 1•5 years ago
|
||
Hey James, thanks for the reply on Bug 1066323! I'd like to work on the above to begin with, since there's an abundance of people working on the others. Thank you!
Cheers.
|
|
Assignee | |
Comment 2•5 years ago
|
||
Hi James, just wanted to clarify and make sure I am on the right track.
You would like capture_stdio in
https://searchfox.org/mozilla-central/source/testing/web-platform/tests/tools/wptrunner/wptrunner/testrunner.py
to be added, similarly, to
https://searchfox.org/mozilla-central/source/testing/mozbase/mozlog/mozlog/structuredlog.py
?
The stdio capture used in TestRunner could be added in a similar way to the LoggerState, is that correct?
Please let me know if I am way off and need a little nudge in a different direction!
Thank you in advance!
|
|
Reporter | |
Comment 3•5 years ago
|
||
Hi, sorry for not replying to this earlier, I was afk all weekend.
Yes, the idea is to take the CaptureIO class as used at [1] from [2] and put it into mozlog somewhere along with all the dependecies required to run it (probably in a new file somewhere). You probably also want to try writing some tests to ensure that it is working correctly after the move.
Does that make sense?
[1] https://searchfox.org/mozilla-central/source/testing/web-platform/tests/tools/wptrunner/wptrunner/testrunner.py#141
[2] https://searchfox.org/mozilla-central/source/testing/web-platform/tests/tools/wptrunner/wptrunner/wptlogging.py#103-134
|
|
Assignee | |
Comment 4•5 years ago
|
||
Hi James. No worries! It was the weekend. :)
Thank you for getting back to me. This makes sense. Although, I may have some more specific questions to follow!
Looking at the CaptureIO class, it relies on the LogThread and LoggingWrapper classes that are in the same file. I am wondering if it would make sense to move those along with it?
Also, as testrunner.py[1] already imports part of mozlog anyway, it would be easy to just add the new capture.py file I'm creating in mozlog as another dependency. That would mean, once moved, the CaptureIO class currently in wptlogging.py[2] could be deleted safely to avoid duplication. Does that work?
Thanks again for your help!
|
|
Reporter | |
Comment 5•5 years ago
|
||
yes, I imagine moving all the required code out of the wptlogging file and into mozlog. Then the wptlogging file should just import from mozlog instead of duplicating the code, just as you say.
|
|
Assignee | |
Comment 6•5 years ago
|
||
The ability to capture the parent process' stdio is suggested to be a useful feature
to move from web-platform/tests into mozlog. To do so, I have created a new capture.py
file within mozlog/mozlog. This includes the CaptureIO class and its dependencies,
including the LoggingWrapper and LogThread classes. These have been removed from their
original location, to avoid duplication, and the files depending on them updated
accordingly.
It would be useful to add unittests testing the CaptureIO enter and exit methods, and
the original_stdio, logging_queue and logging_thread properties. I have begun such a
file with test_capture.py in mozlog/tests. This is a work in progress, however I may
need some guidance, please, in regards to creating appropriate mock data to assert.
|
|
Assignee | |
Comment 7•5 years ago
|
||
Hey James! Added what I think will be the solution to this. However, I was stumped on what data to use to test it in the new location. I imagine I will need to test the methods and properties of the CaptureIO class? But am not sure how to mock data to do so. Could you please point me in the right direction? Thanks! Hopefully everything else looks ok so far!
|
|
Reporter | |
Comment 8•5 years ago
|
||
|
|
Reporter | |
Comment 9•5 years ago
|
||
So it seems there are some failing tests here. The Python 3 failures seem to be because StringIO doesn't exist; I think we can use BytesIO in both cases. The Python 2 failures look like things not working as expected on some platforms; possibly existing bugs in the code, and require more investigation.
|
|
Reporter | |
Comment 10•5 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=693fa6df3832ef2d0145fd1d35a68184ee1c878f is a try push with the StringIO to BytesIO conversion. It seems that there are some more Python 3 issues to look at, plus some Python 2 failures on non-linux platforms.
|
|
Assignee | |
Comment 11•5 years ago
|
||
Thanks for looking into it! This is my first time using Treeherder. How do I tell what other Python2/3 discrepancies need fixing? I'm not quite sure what to change here...
Thanks!
|
||
Comment 12•5 years ago
|
||
| bugherder | ||
|
||
Updated•5 years ago
|
|
||
Comment 13•5 years ago
|
||
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/15880 for changes under testing/web-platform/tests
Can't merge web-platform-tests PR due to failing upstream checks: Github PR https://github.com/web-platform-tests/wpt/pull/15880 * Taskcluster (pull_request) (https://tools.taskcluster.net/task-group-inspector/#/dCwkAEu2Tf6xxywdt_wDZg)
Can't merge web-platform-tests PR due to failing upstream checks: Github PR https://github.com/web-platform-tests/wpt/pull/15880 * Taskcluster (pull_request) (https://tools.taskcluster.net/task-group-inspector/#/SJKQhLUeSSW68iWxsHPCTg)
Upstream PR merged
Upstream PR merged
Upstream PR merged
Upstream PR merged
Upstream PR merged
|
||
Comment 22•5 years ago
|
||
Pushed by james@hoppipolla.co.uk: https://hg.mozilla.org/integration/mozilla-inbound/rev/09802bf2857c [wpt PR 15880] - [Gecko Bug 1021926] mozlog: move the capture io class from web-platform/tests to mozlog (bug 1021926), a=testonly
|
||
Comment 23•5 years ago
|
||
| bugherder | ||
|
||
Comment 24•5 years ago
|
||
| bugherder | ||
Description
•