Closed Bug 1341476 Opened 7 years ago Closed 7 years ago

Running mochitests with --dump-dmd-after-test doesn't actually dump DMD output per test

Categories

(Testing :: Mochitest, defect)

defect
Not set
normal

Tracking

(firefox54 fixed)

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: kats, Assigned: kats)

References

Details

Attachments

(1 file, 1 obsolete file)

According to `mach help mochitest`, it is possible to run mochitests with DMD enabled. There is a --dmd option and other related options to get DMD/memory dumps. However it looks like running with --dmd goes through the code at [1], which tries to call a function call DMDAnalyzeReports. This function used to be exposed to JS via nsJSEnvironment.cpp but it was removed in [2]. Therefore this feature is, as we say, "busted".

We should either fix it up and get it working again, or remove it from the mochitest options/help output.

[1] http://searchfox.org/mozilla-central/rev/12cf11303392edac9f1da0c02e3d9ad2ecc8f4d3/testing/mochitest/tests/SimpleTest/MemoryStats.js#120
[2] https://hg.mozilla.org/mozilla-central/rev/a9cbd3f06940#l1.104
There is also a --dump-dmd-after-test option. It may be broken too; I haven't tried it.
Sorry - I made a mistake. Running with --dmd does seem to (at least try to) run with DMD. It's the --dump-dmd-after-test option that's broken (and which I was also running with).

That being said I'm also seeing weirdness with --dmd in that it seems like the parent process spawns another parent process (instead of a child process) and then it just hangs, or something. I'm investigating.
See Also: → 1341450
Summary: Running mochitests with --dmd doesn't actually run with DMD → Running mochitests with --dump-dmd-after-test doesn't actually dump DMD output per test
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #2)
> That being said I'm also seeing weirdness with --dmd in that it seems like
> the parent process spawns another parent process (instead of a child
> process) and then it just hangs, or something. I'm investigating.

This appears to be a deadlock in the DMD code, I'll file another bug for this.
I filed bug 1341621 for the other issue. To get the --dump-dmd-after-test option working again, it looks like the simplest approach would be to re-expose a function to JS that the mochitest harness can call per-test that will dump the DMD output. For now I managed to get useful output by just manually triggering the DMD output using `kill -34`, so this isn't blocking me any more.
Attached patch WIP (obsolete) — Splinter Review
This doesn't quite work but it's really hard for me to make any progress without fixing bug 1341621 first. Stashing this here for now.
Attachment #8840579 - Attachment is obsolete: true
Assignee: nobody → bugmail
Comment on attachment 8840645 [details]
Bug 1341476 - Make the dump-dmd-after-test mochitest option work again.

https://reviewboard.mozilla.org/r/115094/#review116590

::: testing/mochitest/tests/SimpleTest/MemoryStats.js:108
(Diff revision 1)
>          }, null, /* anonymize = */ false);
>      }
>  
> +    if (dumpDMD) {
> -    // This is the old, deprecated function.
> +        // This is the old, deprecated function.
> -    if (dumpDMD && typeof(DMDReportAndDump) != "undefined") {
> +        if (typeof(DMDReportAndDump) != "undefined") {

who uses the old ways?  They still report to the dumpOutputDirectory.  I am confused based on the cli params message changed in this patch and the if/elif blocks here.
Attachment #8840645 - Flags: review?(jmaher) → review-
(In reply to Joel Maher ( :jmaher) from comment #7)
> who uses the old ways?  They still report to the dumpOutputDirectory.  I am
> confused based on the cli params message changed in this patch and the
> if/elif blocks here.

That's a good point. I think we can just get rid of the old ways entirely. I wasn't sure if we somehow needed it for backwards compatibility with older builds or something. But I guess we should never be running older builds with a newer mochitest harness, right?
if we need an older build, we should be getting the harness and tests from the same (or similar) revision.  I am all for now and the future.
Comment on attachment 8840645 [details]
Bug 1341476 - Make the dump-dmd-after-test mochitest option work again.

https://reviewboard.mozilla.org/r/115094/#review116592

thanks for the update!
Attachment #8840645 - Flags: review?(jmaher) → review+
Pushed by kgupta@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/165111e8940e
Make the dump-dmd-after-test mochitest option work again. r=jmaher
https://hg.mozilla.org/mozilla-central/rev/165111e8940e
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
You need to log in before you can comment on or make changes to this bug.