Closed Bug 743026 Opened 10 years ago Closed 10 years ago

runxpcshelltests.py --verbose should show the full command actually run, the environment, and the working directory

Categories

(Testing :: XPCShell Harness, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla14

People

(Reporter: jimb, Assigned: jimb)

Details

Attachments

(1 file)

For debugging, it's really nice to be able to get the xpcshell invocation that runxpcshelltests.py is actually making. This includes the environment variables it sets, and the directory it decides to run the tests in.
Attachment #612707 - Flags: review?(khuey)
Assignee: nobody → jimb
Comment on attachment 612707 [details] [diff] [review]
Make the --verbose flag to runxpcshelltests.py also show the command, environment, and cwd.

I'm not a testing peer.
Attachment #612707 - Flags: review?(khuey) → review?(ted.mielczarek)
Comment on attachment 612707 [details] [diff] [review]
Make the --verbose flag to runxpcshelltests.py also show the command, environment, and cwd.

Review of attachment 612707 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not totally sold on this concept. Do you think this is always useful, or could we get away with only printing this information in the case of test failures? Presumably you care about being able to reproduce a failing test, or digging into the root causes?

::: testing/xpcshell/runxpcshelltests.py
@@ +662,5 @@
> +            # Show only those environment variables that are changed from
> +            # the ambient environment.
> +            changedEnv = (set("%s=%s" % i for i in self.env.iteritems())
> +                          - set("%s=%s" % i for i in os.environ.iteritems()))
> +            self.log.info("TEST-INFO | %s | environment: %s" % (name, list(changedEnv)))

This is clever, but do you think it will be a bit confusing, since you could have an environment variable set outside the script influencing the results, but it wouldn't be printed here?
(In reply to Ted Mielczarek [:ted] from comment #2)
> I'm not totally sold on this concept. Do you think this is always useful, or
> could we get away with only printing this information in the case of test
> failures? Presumably you care about being able to reproduce a failing test,
> or digging into the root causes?

Well, at times I want to reproduce tests that aren't failing. It only prints when --verbose is given; that's showing all the tests' output, too. --verbose isn't the default. Isn't that okay?

> ::: testing/xpcshell/runxpcshelltests.py
> @@ +662,5 @@
> > +            # Show only those environment variables that are changed from
> > +            # the ambient environment.
> > +            changedEnv = (set("%s=%s" % i for i in self.env.iteritems())
> > +                          - set("%s=%s" % i for i in os.environ.iteritems()))
> > +            self.log.info("TEST-INFO | %s | environment: %s" % (name, list(changedEnv)))
> 
> This is clever, but do you think it will be a bit confusing, since you could
> have an environment variable set outside the script influencing the results,
> but it wouldn't be printed here?

That's true. At first I had it just dump everything, but that's ginormous, and actually really unhelpful when I need to find out what to 'set env' in GDB. I have to do the diff by hand. My goal is to be able to run these tests in GDB in Emacs, so what I really want to know is the difference between the environment xpcshell sees and the one in my Emacs --- and that's clearly out of reach, unless I pair this patch with an Archer extension. That makes it a lot less useful for everyone else.

If you think it's better to print the entire environment, I'll go along with that.
Comment on attachment 612707 [details] [diff] [review]
Make the --verbose flag to runxpcshelltests.py also show the command, environment, and cwd.

Review of attachment 612707 [details] [diff] [review]:
-----------------------------------------------------------------

::: testing/xpcshell/runxpcshelltests.py
@@ +657,3 @@
>        try:
>          self.log.info("TEST-INFO | %s | running test ..." % name)
> +        if verbose:

Ah, I missed the verbose check here. Given that, that seems fine.
Attachment #612707 - Flags: review?(ted.mielczarek) → review+
Try (xpcshell tests only): https://tbpl.mozilla.org/?tree=Try&rev=627293a7dbba
Try run for 627293a7dbba is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=627293a7dbba
Results (out of 29 total builds):
    success: 27
    warnings: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jblandy@mozilla.com-627293a7dbba
https://hg.mozilla.org/integration/mozilla-inbound/rev/a1f8356394fc
Flags: in-testsuite-
Target Milestone: --- → mozilla14
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/a1f8356394fc
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.