1076787 - Enable wpt in debug configurations

Status: RESOLVED FIXED

(Testing :: web-platform-tests, defect)

Description

[James Graham [:jgraham]]

At the moment this is blocked by failing/intermittent tests. We may simply need to disable some extra tests until they are fixed.

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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.

Comment 2

[James Graham [:jgraham]]

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.

Comment 3

[James Graham [:jgraham]]

Attachment: wpt-debug-expected.diff

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8d8ecb2498c9

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8d8ecb2498c9

Comment 7

[James Graham [:jgraham]]

Sorry, should have marked this as leave-open or whatever it is; that patch is necessary but not sufficient.

Comment 8

[James Graham [:jgraham]]

Attachment: meta-update.diff

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/43ac34bcae77

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/43ac34bcae77

Comment 11

[James Graham [:jgraham]]

Attachment: wpt-debug.diff

Comment 12

[Jordan Lund (: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?

Comment 13

[James Graham [:jgraham]]

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.

Comment 14

[Jordan Lund (:jlund)]

(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?

Comment 15

[James Graham [:jgraham]]

Attachment: wpt-debug.diff

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.

Comment 16

[Jordan Lund (: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.

Comment 17

[James Graham [:jgraham]]

Attachment: wpt-debug.diff

It's possible I just uploaded the wrong patch before. I'll also add the output of list_builder_differences.sh for this patch.

Comment 18

[James Graham [:jgraham]]

Attachment: differences.txt

Comment 19

[Jordan Lund (:jlund)]

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

Comment 20

[James Graham [:jgraham]]

url:        https://hg.mozilla.org/build/buildbot-configs/rev/eb78c2343dc4dee944e5744132d3565ab1ef17ea
changeset:  eb78c2343dc4dee944e5744132d3565ab1ef17ea
user:       James Graham <[e-mail]>
date:       Thu Jul 16 13:57:43 2015 +0100
description:
Bug 1076787 - Enable web-platform-tests debug on remaining non-try trees, r=jlund

Comment 21

[Jordan Lund (:jlund)]

In production: https://hg.mozilla.org/build/buildbot-configs/rev/eb78c2343dc4