Enable wpt in debug configurations

RESOLVED FIXED in Firefox 41

Status

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: jgraham, Assigned: jgraham)

Tracking

unspecified
mozilla41
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(4 attachments, 2 obsolete attachments)

At the moment this is blocked by failing/intermittent tests. We may simply need to disable some extra tests until they are fixed.
What's the status here? I'd like to remove imptests, but I don't think I should do that before we have debug coverage.
Flags: needinfo?(james)
The status is basically unchanged. I need to take a closer look at the unstable ones to see if there's a pattern, or if we are going to have every test failing 1% of the time due to slow timers or whatever.
Flags: needinfo?(james)
Depends on: 1170645, 1149815, 1170252
Attachment #8616082 - Flags: review?(Ms2ger)
Comment on attachment 8616082 [details] [diff] [review]
wpt-debug-expected.diff

r=me, but please file a bug somewhere with the list of tests you disabled.
Attachment #8616082 - Flags: review?(Ms2ger) → review+
https://hg.mozilla.org/mozilla-central/rev/8d8ecb2498c9
Assignee: nobody → james
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Sorry, should have marked this as leave-open or whatever it is; that patch is necessary but not sufficient.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #8625311 - Flags: review?(Ms2ger)
Attachment #8625311 - Flags: review?(Ms2ger) → review+
https://hg.mozilla.org/mozilla-central/rev/43ac34bcae77
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Posted patch wpt-debug.diff (obsolete) — Splinter Review
Attachment #8634689 - Flags: review?(jlund)
Comment on attachment 8634689 [details] [diff] [review]
wpt-debug.diff

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

sorry for the delay. Looks good but a couple questions. f+ for now until those questions are answered. Feel free to reset the r? when you are ready.


do we still need this logic? http://mxr.mozilla.org/build/source/buildbot-configs/mozilla-tests/config.py#2351

::: mozilla-tests/config.py
@@ +2374,5 @@
>          if not (platform in ('linux64', 'linux', 'win32', 'win64') or
>                  (platform == "macosx64" and slave_platform != "snowleopard")):
>              BRANCHES['cedar']['platforms'][platform][slave_platform]['opt_unittest_suites'] += WEB_PLATFORM_REFTESTS[:]
>              BRANCHES['cedar']['platforms'][platform][slave_platform]['opt_unittest_suites'] += WEB_PLATFORM_TESTS_CHUNKED[:]
> +            BRANCHES['cedar']['platforms'][platform][slave_platform]['debug_unittest_suites'] += WEB_PLATFORM_TESTS_CHUNKED_MORE[:] + WEB_PLATFORM_REFTESTS

bear with me, what's happening here? The block you are indenting into seems to be a special case for a few platforms where we chunk less for wpt? How come we are moving the debug tests into this block and keeping the '_MORE' chunking? How come we still need the debug line at all?

iiuc - won't you "# Enable wpt debug for gecko >= 42" block above add WPT debug tests for every branch + platform including in cedar?
Attachment #8634689 - Flags: review?(jlund) → feedback+
There are still a couple of differences between "everything" (i.e. what's on cedar) and "everything on inbound"; specifically OSX 10.6 and Linux ASAN. I don't really care about 10.6 because I don't ever expect that to be supported, but greening up and enabling ASAN might happen one day.
(In reply to James Graham [:jgraham] from comment #13)
> There are still a couple of differences between "everything" (i.e. what's on
> cedar) and "everything on inbound"; specifically OSX 10.6 and Linux ASAN. I
> don't really care about 10.6 because I don't ever expect that to be
> supported, but greening up and enabling ASAN might happen one day.

k, I think I'm with you. But I still think that:
> +            BRANCHES['cedar']['platforms'][platform][slave_platform]['debug_unittest_suites'] += WEB_PLATFORM_TESTS_CHUNKED_MORE[:] + WEB_PLATFORM_REFTESTS

will add a duplicate builder already existing on cedar since you have enabled wpt debug for gecko >= 42 (which would include cedar) no?
Posted patch wpt-debug.diff (obsolete) — Splinter Review
I double checked using braindump, and this really doesn't add any extra builders on cedar; the change just puts the debug tests in the same if statement that's being used to prevent double-builders for the opt tests.

I did realise, however, that I could remove the special case for Try now.
Attachment #8634689 - Attachment is obsolete: true
Attachment #8636113 - Flags: review?(jlund)
Comment on attachment 8636113 [details] [diff] [review]
wpt-debug.diff

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

Interesting, what command(s) were you running? I downloaded his patch and confirmed to get:
Traceback (most recent call last):
  File "/Users/jlund/devel/mozilla/dev_master/lib/python2.6/site-packages/buildbot-0.8.2_hg_56bbd2b06325_default-py2.6.egg/buildbot/scripts/runner.py", line 1042, in doCheckConfig
    ConfigLoader(configFileName=configFileName)
  File "/Users/jlund/devel/mozilla/dev_master/lib/python2.6/site-packages/buildbot-0.8.2_hg_56bbd2b06325_default-py2.6.egg/buildbot/scripts/checkconfig.py", line 31, in __init__
    self.loadConfig(configFile, check_synchronously_only=True)
  File "/Users/jlund/devel/mozilla/dev_master/lib/python2.6/site-packages/buildbot-0.8.2_hg_56bbd2b06325_default-py2.6.egg/buildbot/master.py", line 812, in loadConfig
    % b['name'])
ValueError: duplicate builder name Rev4 MacOSX Snow Leopard 10.6 cedar debug test web-platform-tests-1
make: *** [checkconfig] Error 1
~/bin/dotfiles/mozilla/verify_branch.sh -e test -f wpt-debug2  105.16s user 3.09s system 96% cpu 1:51.84 total



from what I can tell, I see you doing:


+    for name, branch in items_at_least(BRANCHES, 'gecko_version', 42):
+        for slave_platform in PLATFORMS[platform]['slave_platforms']:
+            if platform in BRANCHES[name]['platforms']:
+                if slave_platform in BRANCHES[name]['platforms'][platform]:
+                    BRANCHES[name]['platforms'][platform][slave_platform]['debug_unittest_suites'] += WEB_PLATFORM_TESTS_CHUNKED_MORE[:] + WEB_PLATFORM_REFTESTS

and then:

     for slave_platform in PLATFORMS[platform]['slave_platforms']:
         if slave_platform not in BRANCHES['cedar']['platforms'][platform]:
             continue
 
         if not (platform in ('linux64', 'linux', 'win32', 'win64') or
                 (platform == "macosx64" and slave_platform != "snowleopard")):
             # ...
             # etc
            BRANCHES['cedar']['platforms'][platform][slave_platform]['debug_unittest_suites'] += WEB_PLATFORM_TESTS_CHUNKED_MORE[:] + WEB_PLATFORM_REFTESTS

the point is, in both cases you are going over every platform: `for slave_platform in PLATFORMS[platform]['slave_platforms']:`. So even if on the cedar one, you reduce the list to a sublist (i.e. snowleopard), you are still within the same list and you are adding the same builders: `+= WEB_PLATFORM_TESTS_CHUNKED_MORE[:] + WEB_PLATFORM_REFTESTS`

I *think* this works for things like: http://hg.mozilla.org/build/buildbot-configs/file/34925337358e/mozilla-tests/config.py#l2368 because you explicitly ignore snowleopard for the opt wpt platforms: http://hg.mozilla.org/build/buildbot-configs/file/34925337358e/mozilla-tests/config.py#l2231

Are you sure you want to add wpt debug to snowleopard for every 42 branch? If so, then I don't think you need the cedar line at all.
Attachment #8636113 - Flags: review?(jlund) → review-
It's possible I just uploaded the wrong patch before. I'll also add the output of list_builder_differences.sh for this patch.
Attachment #8636113 - Attachment is obsolete: true
Attachment #8639214 - Flags: review?(jlund)
Posted file differences.txt
Comment on attachment 8639214 [details] [diff] [review]
wpt-debug.diff

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

\o/ lgtm

::: mozilla-tests/config.py
@@ -2346,5 @@
>              continue
>          BRANCHES['try']['platforms'][platform][slave_platform]['debug_unittest_suites'] += CPP_GTEST
>          BRANCHES['try']['platforms'][platform][slave_platform]['opt_unittest_suites'] += CPP_GTEST
>  
> -# Enable web-platform-tests-debug on try

so now that we have added snowleopard to our list of ignored platforms in the above: '# Enable wpt debug for gecko >= 42', this means that removing this try block will remove snowleopard wpt-debug tests from try jobs. If you want to test debug wpt on try for snowleopard, you will need to do the inverse of your above '# These are not stable enough on OS X 10.6' logic. If this is not a big deal, we can just land this as is and do a follow up
Attachment #8639214 - Flags: review?(jlund) → review+
url:        https://hg.mozilla.org/build/buildbot-configs/rev/eb78c2343dc4dee944e5744132d3565ab1ef17ea
changeset:  eb78c2343dc4dee944e5744132d3565ab1ef17ea
user:       James Graham <james@hoppipolla.co.uk>
date:       Thu Jul 16 13:57:43 2015 +0100
description:
Bug 1076787 - Enable web-platform-tests debug on remaining non-try trees, r=jlund
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.