801682 - Enable media crashtests on the alder branch (and disable leak checks)

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Crashtests for /dom/media/ can't be checked in right now due to leakage. Given that we will have to do the following:

* We need a patch which enables the tests. This has to happen in testing/crashtest/crashtests.list by adding:

"include ../../dom/media/tests/crashtests/crashtests.list"

Keep in mind that my first crashtest on bug 798873 has to land first.

* Disable the processing of the leak log:

http://mxr.mozilla.org/mozilla-central/source/layout/tools/reftest/runreftest.py#136

Eric, can you take that?

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I will take care once my patch on bug 798873 is on alder.

Comment 2

[Jason Smith [:jsmith]]

Most of work is happening on central now, right, given that we did the large import of the code now? Is this really essential now that most development is happening on central?

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

We can't run crashtests at all on m-c due to the leak situation. So we need a way to get those executed. This was discussed in detail yesterday on IRC and is the only viable option we have. As long as we have those leaks we should continue with merging from m-c to the alder branch. The tests for the branch builds will show us if something regressed.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Can anyone please merge m-c into alder so we can get this implemented? I will have a patch ready in a bit but it will not apply until the latest code from m-c is on the alder branch.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Due to the change in runreftests.py we actually should never merge this patch into m-c.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v1

For testing purposes I have pushed it to try for the m-c branch:
https://tbpl.mozilla.org/?tree=Try&rev=ec267581775c

Comment 7

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 672207 [details] [diff] [review]
Patch v1

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

::: layout/tools/reftest/runreftest.py
@@ +133,5 @@
>                                   # give the JS harness 30 seconds to deal
>                                   # with its own timeouts
>                                   timeout=options.timeout + 30.0)
> +      # Disable leak checks until we have time to fix those leaks
> +      # processLeakLog(self.leakLogFile, options.leakThreshold)

Add a comment that this should not get to m-c

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v1.1

Updated patch with the comment added. Taking over r+ from last version.

Randell, do you have any update regarding how you want to handle merges from m-c to alder? When can we land this patch? Would you mind doing so? I do not have a local alder checkout present anymore. Thanks!

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 675107 [details] [diff] [review]
Patch v1.1

This has been landed today:
https://hg.mozilla.org/projects/alder/rev/67ebbc2cf0ff

Randell, shall we keep the bug open for a likely backout later?

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

Leave open for eventual landing on m-c
At some point we'll likely re-image alder rather than back things out.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Randell Jesup [:jesup] from comment #10)
> Leave open for eventual landing on m-c
> At some point we'll likely re-image alder rather than back things out.

The patch here should never land on m-c! We would bust the whole reftest run. :) the only option we have here is to backout the patch or re-image as you have proposed.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This bug has been fixed and we don't plan to do anymore work on it. Once we can enable leak checks we will reopen for a partly backout.