Closed
Bug 771578
Opened 11 years ago
Closed 4 years ago
runxpcshelltests.py should be installed in the objdir virtualenv and make use of mozbase packages
Categories
(Testing :: XPCShell Harness, defect)
Testing
XPCShell Harness
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: k0scist, Unassigned)
References
(Blocks 1 open bug)
Details
(Whiteboard: [mozbase][leave open])
Attachments
(3 files)
104.09 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
21.04 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
19.88 KB,
patch
|
ted
:
feedback+
|
Details | Diff | Splinter Review |
http://mxr.mozilla.org/mozilla-central/source/testing/xpcshell/runxpcshelltests.py http://mxr.mozilla.org/mozilla-central/source/testing/xpcshell/runxpcshelltests.py#19 #TODO: replace this with json.loads when Python 2.6 is required. We can actually do this today because we have simplejson installed in the virtualenv We can also replace the modules needed for runxpcshelltests.py since they are in the virtualenv: http://mxr.mozilla.org/mozilla-central/source/testing/xpcshell/Makefile.in mozinfo is also magically imported and probably won't work without running through `make` If we make use of the objdir virtualenv we can get all of these things for practically free: http://mxr.mozilla.org/mozilla-central/source/build/virtualenv/Makefile.in Though it might be worth blocking on getting automationutils.py (etc) working via mozbase
Comment 1•11 years ago
|
||
I don't think there's any value in installing the file itself into the virtualenv, really. I do agree that it should start using the modules from the virtualenv, but we first need to start using the virtualenv when running packaged tests. (There's a bug with a mozharness script for this.)
Summary: xpcshell should be installed in the objdir virtualenv and make use of mozbase packages → runxpcshelltests.py should be installed in the objdir virtualenv and make use of mozbase packages
Reporter | ||
Updated•11 years ago
|
Whiteboard: [mozbase]
Comment 2•10 years ago
|
||
Bug 804648 will be very difficult without mozprocess. So, adding this as a prerequisite.
Blocks: 804648
Comment 3•10 years ago
|
||
I will be giving the Python files a lot of love before I get to the meat of this bug: making runxpcshelltests.py use mozbase. For this patch, I simply ran reindent.py on all the .py files in /testing/xpcshell.
Comment 4•10 years ago
|
||
This code establishes a standalone function for running a single test. I've wanted to do this for a long, long time. If you don't think this patch by itself warrants being done, you may want to wait to see subsequent patches which build on top of this.
Attachment #716063 -
Flags: review?(ted)
Comment 5•10 years ago
|
||
Comment on attachment 716062 [details] [diff] [review] Part 1: Reindent Python files, v1 Review of attachment 716062 [details] [diff] [review]: ----------------------------------------------------------------- I'm not actually reviewing this, so rs=me.
Attachment #716062 -
Flags: review?(ted) → review+
Comment 6•10 years ago
|
||
Might as well get the bit rot out of the way sooner. Part 1: https://hg.mozilla.org/integration/mozilla-inbound/rev/ce4f4d4d69d9
Whiteboard: [mozbase] → [mozbase][leave open]
Comment 7•10 years ago
|
||
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/08fb882546da because you put spaces before the "INFO | Passed:" etc. summary at the end of the run, and because we like things fragile and intertwingled, the red/orange/green results of a job are tied to grovelling through the log and matching those with a regex, a regex which it seems did not expect " INFO".
Comment 8•10 years ago
|
||
This patch does a lot. It is incomplete, so only asking for f? at this time. The biggest change in this patch is the switch to mozprocess (instead of raw Popen). Because mozprocess pushes output via callbacks (as opposed to the pull-based model of Popen where you need to call communicate()), this change warranted some additional refactoring around how process execution is performed. We now have a ProcessWrapper class that provides a nice, clean API around process execution. It replaces APIs on the main xpcshell runner class. I've only implemented the default/local class so far. We obviously need separate classes for remote execution as well as B2G, I believe. These will come in a future version of the patch. While I was changing things from pull to push, I also took the opportunity to introduce a class to represent an individual test result. This work is not yet complete. I'm not sure whether I'll leave it in a half-baked state and finish it with a subsequent part or just do it all in this part. I'm leaning towards the former. Since mozprocess supports timeouts easily, I added a timeout for local test running. This is part of bug 597064. I may leave timeouts partially implemented then we can finish the work in that bug. A nice change resulting from the code refactoring is the output buffering behavior. Since mozprocess is line buffered, if a local process is aborted, the test result class has all of the output already. No additional process/pipe/buffering mucking is needed. This will make bug 804648 much easier to implement. Another side-effect of this is that we don't buffer test output in certain modes. So, now when you run a single xpcshell test with mach, output is printed as the child process emits it. Previously, all output was buffered until the child process exited and then mach printed it. I like the new model. This patch paves the way for additional refactoring of the xpcshell runner. Eventually, I want there to be a XPCShellTest class that encapsulates test metadata and provides mechanisms for running tests and interpreting the results. Once this is done, the test runner will simply instantiate multiple instances then proceed to run them, eventually in parallel. This will require a lot of additional work because currently individual tests alter state of the global test runner. We'll get there. For the feedback review, I'm mostly interested in comments on patch direction. If I'm doing something wrong or doing things that overlap with other efforts, please let me know.
Attachment #716321 -
Flags: feedback?(ted)
Comment 9•10 years ago
|
||
(In reply to Phil Ringnalda (:philor) from comment #7) > Backed out in > https://hg.mozilla.org/integration/mozilla-inbound/rev/08fb882546da because > you put spaces before the "INFO | Passed:" etc. summary at the end of the > run, and because we like things fragile and intertwingled, the > red/orange/green results of a job are tied to grovelling through the log and > matching those with a regex, a regex which it seems did not expect " > INFO". Well, I guess reindent.py is buggy. Sad panda. Perhaps we should have unit tests that ensure the printed output is sane...
Comment 10•10 years ago
|
||
There was a multiline log string that got indented. I changed this to be a series of individual log messages and verified locally the output looks sane. IMO the trivial change did not warrant explicit review. https://hg.mozilla.org/integration/mozilla-inbound/rev/43af5bd72395
Comment 11•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/43af5bd72395
Flags: in-testsuite+
Comment 12•10 years ago
|
||
Comment on attachment 716321 [details] [diff] [review] Part 3: Use mozprocess for running tests, v1 Review of attachment 716321 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/xpcshell/runxpcshelltests.py @@ +71,5 @@ > + > + # Environment variables used everywhere. > + self._core_environment = { > + > + } Something missing?
Updated•10 years ago
|
Attachment #716063 -
Flags: review?(ted) → review+
Comment 13•10 years ago
|
||
Part 2: https://hg.mozilla.org/integration/mozilla-inbound/rev/2177de4cfb4f
Comment 15•10 years ago
|
||
Comment on attachment 716321 [details] [diff] [review] Part 3: Use mozprocess for running tests, v1 Review of attachment 716321 [details] [diff] [review]: ----------------------------------------------------------------- Sorry, I keep forgetting this was in my queue (and you never bug me about it). Overall this looks nice. Despite the fact that I only wrote this harness about 6 years ago it's grown pretty crufty. ::: testing/xpcshell/runxpcshelltests.py @@ +37,5 @@ > +class ProcessWrapper(object): > + """Wrapper for interacting with processes. > + > + The default implementation provides a facility for local processes. Derived > + classes may support remote processes, etc. Is it just me, or are you just working around a deficient mozprocess API? We talked about making the mozprocess API suck less, I wonder if you'd have any input there. @@ -907,5 @@ > - # if e10s test started but never finished (child process crash) > - (stdout and re.search("^child: CHILD-TEST-STARTED", > - stdout, re.MULTILINE) > - and not re.search("^child: CHILD-TEST-COMPLETED", > - stdout, re.MULTILINE))) Cleaning this up is really nice!
Attachment #716321 -
Flags: feedback?(ted) → feedback+
Updated•9 years ago
|
Assignee: gps → nobody
Comment 17•4 years ago
|
||
No assignee, updating the status.
![]() |
||
Comment 18•4 years ago
|
||
Unfortunately, I'm not sure the wip here can be salvaged.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•