Last Comment Bug 793022 - roll out mozharness unittests to mozilla-central + project branches when ready
: roll out mozharness unittests to mozilla-central + project branches when ready
Status: RESOLVED FIXED
[mozharness][unittest]
:
Product: Release Engineering
Classification: Other
Component: Mozharness (show other bugs)
: other
: All All
: -- normal (vote)
: ---
Assigned To: Aki Sasaki [:aki]
: Chris AtLee [:catlee]
Mentors:
Depends on: 650880 793017 795525 795533 795535 802796 805933 806524 806651 807094 808536 808814 812723 812726 812727 821445 826472 837695 838325 838485 838492 839707 840202 840309 840694 843464 843467
Blocks: 775756 746243 746246 843950
  Show dependency treegraph
 
Reported: 2012-09-20 15:40 PDT by Aki Sasaki [:aki]
Modified: 2014-07-09 10:11 PDT (History)
14 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
enable mozharness unittests on ash (aurora mirror) (754 bytes, patch)
2012-11-12 19:42 PST, Aki Sasaki [:aki]
armenzg: review+
aki: checked‑in+
Details | Diff | Splinter Review
(configs) part 1: get mozilla-tests/config.py to not warn in python-mode (38.32 KB, patch)
2013-01-28 10:05 PST, Aki Sasaki [:aki]
rail: review+
aki: checked‑in+
Details | Diff | Splinter Review
(tools) production-masters.json limit_mobile_platforms (16.76 KB, patch)
2013-01-31 16:47 PST, Aki Sasaki [:aki]
rail: review+
aki: checked‑in+
Details | Diff | Splinter Review
(braindump) allow for -j in dump_masters.sh (738 bytes, patch)
2013-01-31 20:50 PST, Aki Sasaki [:aki]
catlee: review+
aki: checked‑in+
Details | Diff | Splinter Review
(custom) remove enable_mobile_unittests support (1.04 KB, patch)
2013-01-31 23:48 PST, Aki Sasaki [:aki]
rail: review+
aki: checked‑in+
Details | Diff | Splinter Review
(configs) part 2: split out mozilla-tests/mobile_config.py (99.65 KB, patch)
2013-02-06 16:24 PST, Aki Sasaki [:aki]
rail: review-
Details | Diff | Splinter Review
(configs) part 2, take 2: split out mozilla-tests/mobile_config.py (88.63 KB, patch)
2013-02-07 16:44 PST, Aki Sasaki [:aki]
no flags Details | Diff | Splinter Review
(configs) part 2, take 3: split out mozilla-tests/mobile_config.py (111.46 KB, patch)
2013-02-07 16:57 PST, Aki Sasaki [:aki]
rail: review-
Details | Diff | Splinter Review
(configs) some fixes (6.69 KB, patch)
2013-02-08 08:49 PST, Rail Aliiev [:rail]
no flags Details | Diff | Splinter Review
(configs) part 2, take 4: split out mozilla-tests/mobile_config.py (87.33 KB, patch)
2013-02-08 10:57 PST, Aki Sasaki [:aki]
rail: review+
aki: checked‑in+
Details | Diff | Splinter Review
(configs) part 2.5: clean up mozilla-tests/thunderbird_config.py (13.56 KB, patch)
2013-02-08 19:51 PST, Aki Sasaki [:aki]
jhopkins: review+
aki: checked‑in+
Details | Diff | Splinter Review
(mozharness) make the desktop_unittest.py --suite options extend (1.82 KB, patch)
2013-02-13 22:51 PST, Aki Sasaki [:aki]
bhearsum: review+
aki: checked‑in+
Details | Diff | Splinter Review
(custom) make config refactor a little less ugly (3.96 KB, patch)
2013-02-13 22:59 PST, Aki Sasaki [:aki]
no flags Details | Diff | Splinter Review
(configs) part 3: roll out mozharness desktop unittests (52.64 KB, patch)
2013-02-13 23:09 PST, Aki Sasaki [:aki]
bhearsum: review+
Details | Diff | Splinter Review
(custom) make config refactor a little less ugly, also use deepcopy() (4.78 KB, patch)
2013-02-13 23:31 PST, Aki Sasaki [:aki]
bhearsum: review+
aki: checked‑in+
Details | Diff | Splinter Review
before/after differences in config.py (123.45 KB, application/x-gzip-compressed)
2013-02-14 10:00 PST, Ben Hearsum (:bhearsum)
no flags Details
(configs) fix ubuntu (1013 bytes, patch)
2013-02-19 18:32 PST, Aki Sasaki [:aki]
rail: review+
Details | Diff | Splinter Review
(configs) unbitrotted, with ubuntu fix (50.24 KB, patch)
2013-02-19 18:33 PST, Aki Sasaki [:aki]
aki: review+
aki: checked‑in+
Details | Diff | Splinter Review
stop trying to fetch symbols for b2g tests (862 bytes, patch)
2013-02-20 11:49 PST, Aki Sasaki [:aki]
rail: review+
aki: checked‑in+
Details | Diff | Splinter Review

Description Aki Sasaki [:aki] 2012-09-20 15:40:27 PDT
We should find and fix issues on Cedar first.
Comment 1 Aki Sasaki [:aki] 2012-10-29 15:54:55 PDT
We're a reconfig, a backout, and a final look-over away from being unblocked here.  (Read: ready to roll out to m-c and project branches.)

Are there anything else we want to look at or fix before that happens?
Comment 2 Aki Sasaki [:aki] 2012-10-29 15:59:10 PDT
Let's also get the DesktopUnittestOutputParser patch from bug 774535 reviewed+landed first.
Comment 3 Aki Sasaki [:aki] 2012-10-30 15:28:37 PDT
It would be nice to solve bug 807094 and bug 758975 before this rolls out, but it's not necessarily a hard block.
Comment 4 Aki Sasaki [:aki] 2012-10-30 15:29:16 PDT
I suppose bug 793642 is what bug 758975 is duped to.
Comment 5 Aki Sasaki [:aki] 2012-11-12 10:45:05 PST
I'm going to start the ball rolling to get these unittests live this week -- any objections?
Comment 6 Aki Sasaki [:aki] 2012-11-12 19:42:31 PST
Created attachment 680929 [details] [diff] [review]
enable mozharness unittests on ash (aurora mirror)
Comment 7 Aki Sasaki [:aki] 2012-11-13 09:47:14 PST
Comment on attachment 680929 [details] [diff] [review]
enable mozharness unittests on ash (aurora mirror)

http://hg.mozilla.org/build/buildbot-configs/rev/c50d11fe5ce9
Comment 8 Aki Sasaki [:aki] 2012-11-15 19:42:28 PST
Now all I need is time to write + test the buildbot-configs patch, and roll out without too much weekend/holiday risk.

This may take a backseat to b2g work for a bit, but it's still very much on my radar.
Comment 9 Aki Sasaki [:aki] 2013-01-28 10:05:42 PST
Created attachment 707161 [details] [diff] [review]
(configs) part 1: get mozilla-tests/config.py to not warn in python-mode

I can wait to land til after your AWS test config patches and rebase if needed.
Comment 10 Aki Sasaki [:aki] 2013-01-28 10:41:37 PST
Comment on attachment 707161 [details] [diff] [review]
(configs) part 1: get mozilla-tests/config.py to not warn in python-mode

http://hg.mozilla.org/build/buildbot-configs/rev/97d8757436a9
Comment 11 Aki Sasaki [:aki] 2013-01-31 16:47:50 PST
Created attachment 708850 [details] [diff] [review]
(tools) production-masters.json limit_mobile_platforms

Needs the corresponding configs (part 2) patch.
Comment 12 Aki Sasaki [:aki] 2013-01-31 20:50:38 PST
Created attachment 708935 [details] [diff] [review]
(braindump) allow for -j in dump_masters.sh

-8 is deprecated as well.
Comment 13 Aki Sasaki [:aki] 2013-01-31 23:41:27 PST
WIP:  https://github.com/escapewindow/build-buildbot-configs/compare/master...mobile_config

* I had to create a mobile_platforms since project_branches.py is shared and I'm not creating a mozilla/mobile_config.py =\
* I'm seeing dump_masters.sh differences that shouldn't be there; I'm going to have to go through and figure out why and fix.
Comment 14 Aki Sasaki [:aki] 2013-01-31 23:48:52 PST
Created attachment 708961 [details] [diff] [review]
(custom) remove enable_mobile_unittests support

This is only for Firefox desktop support of mobile unittests.
The only mobile unittest suite we have is "mobile-mochitest-browser-chrome", which was for running mobile unittests against Fennec linux-i686 builds (which haven't been running for a long, long time).

http://hg.mozilla.org/build/buildbot-configs/rev/509c45e5b106
https://bugzilla.mozilla.org/show_bug.cgi?id=511282
Comment 15 Aki Sasaki [:aki] 2013-02-01 10:54:30 PST
Comment on attachment 708935 [details] [diff] [review]
(braindump) allow for -j in dump_masters.sh

http://hg.mozilla.org/build/braindump/rev/1b303076edf3
Comment 16 Aki Sasaki [:aki] 2013-02-05 22:22:45 PST
I successfully split out mozilla-tests/mobile-config.py , albeit from a changeset a week ago (git revision 9d41ad4236bbaa31014b0477c3bf7362b43fe3ca).

The only dump-masters diff is the mobile desktop tests that are obsolete.

Here's the diff:
https://github.com/escapewindow/build-buildbot-configs/compare/master...mobile_config

Now I need to unbitrot.
Comment 17 Aki Sasaki [:aki] 2013-02-06 16:24:30 PST
Created attachment 711058 [details] [diff] [review]
(configs) part 2: split out mozilla-tests/mobile_config.py

I diffed dump_masters.sh so this should be sane.
Comment 18 Rail Aliiev [:rail] 2013-02-07 13:49:50 PST
Comment on attachment 711058 [details] [diff] [review]
(configs) part 2: split out mozilla-tests/mobile_config.py

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

Can you remove unused code and move shared functions somewhere else? See my comments inline.

::: mozilla-tests/mobile_config.py
@@ +17,5 @@
> +                        'release-mozilla-beta': 'org.mozilla.firefox_beta',
> +                        'release-mozilla-release': 'org.mozilla.firefox',
> +                        }
> +
> +MOZHARNESS_REBOOT_CMD = ['scripts/external_tools/count_and_reboot.py',

Not used. Removing it doesn't break test-masters.sh

@@ +23,5 @@
> +                         '-n', '1', '-z']
> +
> +TALOS_CMD = ['python', 'run_tests.py', '--noisy', WithProperties('%(configFile)s')]
> +
> +TALOS_DIRTY_OPTS = {'talosAddOns': ['profiles/dirtyDBs.zip', 'profiles/dirtyMaxDBs.zip']}

the same here

@@ +25,5 @@
> +TALOS_CMD = ['python', 'run_tests.py', '--noisy', WithProperties('%(configFile)s')]
> +
> +TALOS_DIRTY_OPTS = {'talosAddOns': ['profiles/dirtyDBs.zip', 'profiles/dirtyMaxDBs.zip']}
> +
> +TALOS_TP_OPTS = {'plugins': {'32': 'zips/flash32_10_3_183_5.zip', '64': 'zips/flash64_11_0_d1_98.zip'}, 'pagesets': ['zips/tp5.zip']}

the same here

@@ +26,5 @@
> +
> +TALOS_DIRTY_OPTS = {'talosAddOns': ['profiles/dirtyDBs.zip', 'profiles/dirtyMaxDBs.zip']}
> +
> +TALOS_TP_OPTS = {'plugins': {'32': 'zips/flash32_10_3_183_5.zip', '64': 'zips/flash64_11_0_d1_98.zip'}, 'pagesets': ['zips/tp5.zip']}
> +TALOS_TP_NEW_OPTS = {'plugins': {'32': 'zips/flash32_10_3_183_5.zip', '64': 'zips/flash64_11_0_d1_98.zip'}, 'pagesets': ['zips/tp5n.zip']}

the same here

@@ +124,5 @@
> +            platform_config[slave_platform]['try_slaves'] = platform_config[slave_platform]['slaves']
> +
> +ANDROID = PLATFORMS['android']['slave_platforms']
> +
> +ANDROID_ARMV6 = PLATFORMS['android-armv6']['slave_platforms']

the same here

@@ +180,5 @@
> +    },
> +}
> +
> +
> +def removeSuite(suiteName, suiteList):

Not used function.

@@ +200,5 @@
> +            suiteList[i] = (name, suites)
> +    return suiteList
> +
> +
> +def loadDefaultValues(BRANCHES, branch, branchConfig):

I haven't checked if it's the same function as in config.py, but if it is it would be better to move common functions into a separate module (config_common.py?)

@@ +214,5 @@
> +    BRANCHES[branch]['pgo_strategy'] = branchConfig.get('pgo_strategy', None)
> +    BRANCHES[branch]['pgo_platforms'] = branchConfig.get('pgo_platforms', [])
> +
> +
> +def loadCustomTalosSuites(BRANCHES, SUITES, branch, branchConfig):

the same here

@@ +249,5 @@
> +            # Suites that are turned on by default
> +            BRANCHES[branch][suite + '_tests'] = (talosConfig.get(suite, 1), coallesceJobs) + SUITES[suite]['options']
> +
> +
> +def loadTalosSuites(BRANCHES, SUITES, branch):

and here

@@ +264,5 @@
> +            # Suites that are turned on by default
> +            BRANCHES[branch][suite + '_tests'] = (1, coallesceJobs) + SUITES[suite]['options']
> +
> +
> +def nested_haskey(dictionary, *keys):

not used.

I removed the functions and variables I stated above and tests-masters.sh worked fine.
Comment 19 Aki Sasaki [:aki] 2013-02-07 16:44:24 PST
Created attachment 711598 [details] [diff] [review]
(configs) part 2, take 2: split out mozilla-tests/mobile_config.py

Got this all cleaned up, then hit some git rebase weirdness.  I think I'm back to normal.
Comment 20 Aki Sasaki [:aki] 2013-02-07 16:46:19 PST
Comment on attachment 711598 [details] [diff] [review]
(configs) part 2, take 2: split out mozilla-tests/mobile_config.py

Crap, hit some test-masters.sh issues after weird rebase.
Comment 21 Aki Sasaki [:aki] 2013-02-07 16:57:12 PST
Created attachment 711604 [details] [diff] [review]
(configs) part 2, take 3: split out mozilla-tests/mobile_config.py

Ok, I just forgot -j in the previous test-masters.sh run.

Here's the same patch, but with -U9 for context.
I always knew I'd break 100k at some point.
Comment 22 Rail Aliiev [:rail] 2013-02-08 08:48:34 PST
Comment on attachment 711604 [details] [diff] [review]
(configs) part 2, take 3: split out mozilla-tests/mobile_config.py

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

Some minor issues and bad merge/reabse.

::: mozilla-tests/config.py
@@ -980,5 @@
>          'platforms': {
>              'fedora64': {'ext': 'linux-x86_64.tar.bz2', 'debug': True},
>              'fedora': {'ext': 'linux-i686.tar.bz2', 'debug': True},
> -            'ubuntu64': {'ext': 'linux-x86_64.tar.bz2', 'debug': True},
> -            'ubuntu32': {'ext': 'linux-i686.tar.bz2', 'debug': True},

Looks like a bad rebase. The lines above shouldn't be removed.

::: mozilla-tests/mobile_config.py
@@ +606,5 @@
> +        out = pprint.pformat(v)
> +        for l in out.splitlines():
> +            print '%s: %s' % (k, l)
> +
> +    for suite in SUITES:

Can you use the following instead?

for suite in sorted(SUITES):...

I'll attach a patch with changes I made locale on top of your patch.

::: mozilla/config.py
@@ +1322,5 @@
> +    if 'mobile_platforms' in BRANCHES[branch]:
> +        if 'platforms' not in BRANCHES[branch]:
> +            BRANCHES[branch]['platforms'] = BRANCHES[branch]['mobile_platforms']
> +        else:
> +            BRANCHES[branch]['platforms'].update(BRANCHES[branch]['mobile_platforms'])

Can you use deepcopy (will be in the attached patch) for both cases above? Otherwise you end up with structure like this: http://people.mozilla.org/~raliiev/sattap/c3a76c06.png.

::: mozilla/release-fennec-mozilla-release.py
@@ +147,5 @@
>          '--add-locales',
>          '--summary',
>      ]
>  }
> +releaseConfig['enableSigningAtBuildTime'] = False

Bad merge?

::: mozilla/release-fennec-mozilla-release.py.template
@@ +148,5 @@
>          '--package-multi',
>          '--summary',
>      ]
>  }
> +releaseConfig['enableSigningAtBuildTime'] = False

the same here

::: mozilla/staging_release-fennec-mozilla-release.py
@@ +158,5 @@
>  
>  # Staging config
>  releaseConfig['build_tools_repo_path'] = "users/stage-ffxbld/tools"
>  releaseConfig['skip_release_download'] = True
> +releaseConfig['enableSigningAtBuildTime'] = False

and here

::: mozilla/staging_release-fennec-mozilla-release.py.template
@@ +158,5 @@
>  
>  # Staging config
>  releaseConfig['build_tools_repo_path'] = "users/stage-ffxbld/tools"
>  releaseConfig['skip_release_download'] = True
> +releaseConfig['enableSigningAtBuildTime'] = False

and here
Comment 23 Rail Aliiev [:rail] 2013-02-08 08:49:09 PST
Created attachment 711861 [details] [diff] [review]
(configs) some fixes
Comment 24 Aki Sasaki [:aki] 2013-02-08 10:57:58 PST
Created attachment 711915 [details] [diff] [review]
(configs) part 2, take 4: split out mozilla-tests/mobile_config.py

With fixes.  Once I re-rebased, the various enableSigning/ubuntu issues went away.
Comment 25 Rail Aliiev [:rail] 2013-02-08 11:02:32 PST
Comment on attachment 711915 [details] [diff] [review]
(configs) part 2, take 4: split out mozilla-tests/mobile_config.py

woot! the interdiff lgtm!
Comment 26 Aki Sasaki [:aki] 2013-02-08 11:08:09 PST
Comment on attachment 711915 [details] [diff] [review]
(configs) part 2, take 4: split out mozilla-tests/mobile_config.py

http://hg.mozilla.org/build/buildbot-configs/rev/5e29160d702f
Comment 27 Aki Sasaki [:aki] 2013-02-08 11:08:56 PST
Comment on attachment 708850 [details] [diff] [review]
(tools) production-masters.json limit_mobile_platforms

http://hg.mozilla.org/build/tools/rev/14c1e7b65d88
Comment 28 Aki Sasaki [:aki] 2013-02-08 11:10:07 PST
Comment on attachment 708961 [details] [diff] [review]
(custom) remove enable_mobile_unittests support

http://hg.mozilla.org/build/buildbotcustom/rev/43d91fd84700
Comment 29 Aki Sasaki [:aki] 2013-02-08 17:25:57 PST
Working on the 3rd and final set of patches.
Progress here:
https://github.com/escapewindow/build-buildbot-configs/compare/master...mhunit
Comment 30 Aki Sasaki [:aki] 2013-02-08 17:46:12 PST
Hm, it looks like:

a) we only use loadCustomUnittestSuite() and addSuite() for the accessibility branch, and
b) according to https://tbpl.mozilla.org/?tree=Accessibility and http://hg.mozilla.org/projects/accessibility/, we haven't landed anything on the accessibility branch since April 25 of last year, telling me that the branch is dead.

Marco: is the Accessibility branch still needed?
If so, do we need special casing for the snowleopard mochitests on the Accessibility branch still?

If you answer either question "no", this will remove quite a bit of ugliness from my refactor.
Comment 31 Aki Sasaki [:aki] 2013-02-08 19:51:38 PST
Created attachment 712067 [details] [diff] [review]
(configs) part 2.5: clean up mozilla-tests/thunderbird_config.py

I made the mistake of looking at this file while hacking up config.py.
This definitely isn't required for this bug, but I might as well clean this up while I have context.

I:

* made pep8 whitespace fixes
* removed unused variables
* of the four methods here, three were unused
* the fourth method, removeSuites(), was being used to remove the mochitest-a11y suite, which isn't there to begin with.  Also removed.

Passes test-masters.sh; dump_masters.sh shows zero changes between patched and unpatched.
Comment 32 Aki Sasaki [:aki] 2013-02-11 10:16:43 PST
Comment on attachment 711915 [details] [diff] [review]
(configs) part 2, take 4: split out mozilla-tests/mobile_config.py

Followup fix: http://hg.mozilla.org/build/buildbot-configs/rev/1beecffdb58c
Comment 33 David Bolter [:davidb] 2013-02-11 12:54:41 PST
(In reply to Aki Sasaki [:aki] from comment #30)
> Hm, it looks like:
> 
> a) we only use loadCustomUnittestSuite() and addSuite() for the
> accessibility branch, and
> b) according to https://tbpl.mozilla.org/?tree=Accessibility and
> http://hg.mozilla.org/projects/accessibility/, we haven't landed anything on
> the accessibility branch since April 25 of last year, telling me that the
> branch is dead.
> 
> Marco: is the Accessibility branch still needed?

No. We might want to have it in the future but we should give it up for now.
Comment 34 Rail Aliiev [:rail] 2013-02-11 14:07:48 PST
in production
Comment 35 Aki Sasaki [:aki] 2013-02-11 17:01:12 PST
Blocking on bug 837022 since we found env differences between mozharness unittests and buildbot unittests there.  Hopefully I can knock out the three new blockers today so I can continue with the config rewrite.
Comment 36 Aki Sasaki [:aki] 2013-02-11 18:37:47 PST
Comment on attachment 712067 [details] [diff] [review]
(configs) part 2.5: clean up mozilla-tests/thunderbird_config.py

http://hg.mozilla.org/build/buildbot-configs/rev/1dd23223b486
Comment 37 Marco Zehe (:MarcoZ) on PTO until August 15 2013-02-12 00:46:03 PST
(In reply to David Bolter [:davidb] from comment #33)
> (In reply to Aki Sasaki [:aki] from comment #30)
> > Marco: is the Accessibility branch still needed?
> 
> No. We might want to have it in the future but we should give it up for now.

Agreed! I used it when we were trying to get Mac a11y into a workable state. But since that's basically done, that branch is no longer needed. I don't expect any such major work to happen anytime soon that would warrant an extra branch again. Get rid of it. :)
Comment 38 Aki Sasaki [:aki] 2013-02-12 13:24:46 PST
The thunderbird config cleanup (configs part 2.5) is live.  Still working on the 3rd and potentially final config patch.
Comment 39 Aki Sasaki [:aki] 2013-02-12 17:17:23 PST
:bhearsum -
How do you feel about mozilla-tests/b2g_config.py?
Good model, something we should avoid?
The PLATFORM_UNITTEST_VARS are now pretty large, but avoids adding extra programmatic loops.
http://hg.mozilla.org/build/buildbot-configs/file/ba900eddd4df/mozilla-tests/b2g_config.py#l257
Comment 40 Aki Sasaki [:aki] 2013-02-12 22:52:17 PST
The mozharness unittest env stuff was fixed in bug 837022 and bug 840694, so bug 837022 no longer blocks.

Current progress here:
https://github.com/escapewindow/build-buildbot-configs/compare/master...mhunit

X make UNITTEST_SUITES more like b2g_config
X pass test-masters.sh ?
X PLATFORM_UNITTEST_VARS extra_args
X config_file option per PLATFORM_UNITTEST_VARS
X test-masters.sh
_ diff dump_masters
_ debug extra_args
_ deal with esr/aurora/beta/release opt/debug unittest suites
_ deal with marionette
_ deal with ubuntu
_ deal with jetpack
_ test-masters.sh
_ diff dump_masters
_ review
_ land
_ reconfig
Comment 41 Ben Hearsum (:bhearsum) 2013-02-13 14:14:35 PST
(In reply to Aki Sasaki [:aki] from comment #39)
> :bhearsum -
> How do you feel about mozilla-tests/b2g_config.py?
> Good model, something we should avoid?
> The PLATFORM_UNITTEST_VARS are now pretty large, but avoids adding extra
> programmatic loops.
> http://hg.mozilla.org/build/buildbot-configs/file/ba900eddd4df/mozilla-tests/
> b2g_config.py#l257

I didn't look at this in huge depth but it's definitely a pretty big net-win. The MOCHITEST_ONLY things still irk me...but I'm not sure if that's just because they're allcaps, because they remind me of the old configs, or some other reason...so don't worry about that.

➜  ~  python -mthis | sed -n '4~0p'
Explicit is better than implicit.
Comment 42 Aki Sasaki [:aki] 2013-02-13 22:51:06 PST
Created attachment 713809 [details] [diff] [review]
(mozharness) make the desktop_unittest.py --suite options extend

extend is like append, except instead of having to

    --option 1 --option 2 --option 3

you can also [optionally]

    --option 1,2,3
Comment 43 Aki Sasaki [:aki] 2013-02-13 22:59:41 PST
Created attachment 713812 [details] [diff] [review]
(custom) make config refactor a little less ugly

This patch:

* adds an optional "config_files" and "download_symbols" to the mozharness suite config.  config_files is a list that can take multiple files, but I'll have to fix bug 779294 before we can take advantage of that.

I split out config_files so I could have a platform-specific config file per test suite.  We may need to allow for branch-level-specifics at some point; that may be solved by bug 779294.

I split out download_symbols so I didn't have to have a separate suite_config for opt and debug.
Comment 44 Aki Sasaki [:aki] 2013-02-13 23:09:45 PST
Created attachment 713814 [details] [diff] [review]
(configs) part 3: roll out mozharness desktop unittests

I gave up using dump_masters.sh as a useful diffing tool; I was getting false diffs based on builder order.

This patch:

* follows the b2g_config.py model of explicit additive suites rather than a default list of suites + methods to add/remove suites.
* adds marionette as a full fledged unittest suite.  This means it'll get enabled on all m-c level branches instead of the five or so it's currently enabled on.
* gets rid of two of the for loops at the bottom, as well as the removeSuite() method.
* passes test-masters.sh
* builder_list.py shows me a good diff of builder changes for win, mac, and linux test masters.  I'm actually pretty surprised I got this patch ready tonight.

I didn't touch:

* the leopard loop; I think leopard is dying with esr10 on Tuesday.
* the jetpack loop; I think Armen's getting rid of that this week or next with the in-tree jetpack tests.
* the ubuntu loop; I think Rail is busy balancing tests between ubuntu and fedora, and I'd like to allow him to keep doing so easily until we're fully switched over.

There are some assumptions in the above statements, and I still want to do some testing in staging since seeing the large builder diffs made me a little paranoid.  If you want to hold off reviewing til afterwards, that's understandable; I'd like to get the first pass in so I know what needs work sooner.
Comment 45 Aki Sasaki [:aki] 2013-02-13 23:11:22 PST
X make UNITTEST_SUITES more like b2g_config
X pass test-masters.sh ?
X PLATFORM_UNITTEST_VARS extra_args
X config_file option per PLATFORM_UNITTEST_VARS
X test-masters.sh
X diff dump_masters
X debug extra_args (download_symbols)
X deal with esr/aurora/beta/release opt/debug unittest suites
X deal with marionette
X  windows only on try
X  not leopard*
X  central inbound try fx-team services-central -- enabled on all m-c level
_    make sure jgriffin knows about this
X deal with ubuntu -- leave alone?
X deal with jetpack -- leave alone?
X test-masters.sh
X diff builder_list
X rebase
X test-masters.sh
X diff builder_list
X patch
_ review  <-- WE ARE HERE
_ do some spot tests in staging (in parallel)
_  test around try
_  test around release- testers (enable in mozilla-{beta,esr*,release} to test)
_  test around a reconfig mid-test
_ land
_ reconfig
Comment 46 Aki Sasaki [:aki] 2013-02-13 23:31:04 PST
Created attachment 713819 [details] [diff] [review]
(custom) make config refactor a little less ugly, also use deepcopy()

Just noticed this.
The deepcopy() should prevent us from dirtying the configs for later.
Comment 47 Ben Hearsum (:bhearsum) 2013-02-14 07:16:14 PST
Comment on attachment 713809 [details] [diff] [review]
(mozharness) make the desktop_unittest.py --suite options extend

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

This is because you want to be able run multiple types/chunks of the same suite in one invocation, right?
Comment 48 Ben Hearsum (:bhearsum) 2013-02-14 07:29:28 PST
(In reply to Aki Sasaki [:aki] from comment #44)
> There are some assumptions in the above statements, and I still want to do
> some testing in staging since seeing the large builder diffs made me a
> little paranoid.  If you want to hold off reviewing til afterwards, that's
> understandable; I'd like to get the first pass in so I know what needs work
> sooner.

I think it makes sense for me to have a look before you invest a ton of time in testing, otherwise there ends up being an even higher sunk cost. I want to look as this ASAP to avoid holding you up, it would help to know what the expected changes to the builders are first. Eg, I know marionette is expected to be added in a bunch of places, but should I expect other changes to the list of builders for a branch or the details of any builders?
Comment 49 Ben Hearsum (:bhearsum) 2013-02-14 10:00:08 PST
Created attachment 713952 [details]
before/after differences in config.py

Since you're only modifying config.py, I'm using this diff of the output of 'python config.py' before and after your patch to help me review. Thought you might want to look at it too.
Comment 50 Ben Hearsum (:bhearsum) 2013-02-14 10:14:29 PST
Comment on attachment 713814 [details] [diff] [review]
(configs) part 3: roll out mozharness desktop unittests

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

I won't pretend to be able to functionally review this, but I like what I'm seeing. r=me assuming your staging tests pan out.
Comment 51 Aki Sasaki [:aki] 2013-02-14 10:18:36 PST
(In reply to Ben Hearsum [:bhearsum] from comment #47)
> Comment on attachment 713809 [details] [diff] [review]
> (mozharness) make the desktop_unittest.py --suite options extend
> 
> Review of attachment 713809 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> This is because you want to be able run multiple types/chunks of the same
> suite in one invocation, right?

Correct.  In fact, I could do that with append; this just makes the commandline shorter.

(In reply to Ben Hearsum [:bhearsum] from comment #48)
> (In reply to Aki Sasaki [:aki] from comment #44)
> > There are some assumptions in the above statements, and I still want to do
> > some testing in staging since seeing the large builder diffs made me a
> > little paranoid.  If you want to hold off reviewing til afterwards, that's
> > understandable; I'd like to get the first pass in so I know what needs work
> > sooner.
> 
> I think it makes sense for me to have a look before you invest a ton of time
> in testing, otherwise there ends up being an even higher sunk cost. I want
> to look as this ASAP to avoid holding you up, it would help to know what the
> expected changes to the builders are first. Eg, I know marionette is
> expected to be added in a bunch of places, but should I expect other changes
> to the list of builders for a branch or the details of any builders?

Marionette is the only builder change in terms of adding/removing test builders.
There's a ton of UnittestPackagedBuildFactory -> ScriptFactory changes though.

Thanks for the reviews!  Off to test further in staging.
Comment 52 Aki Sasaki [:aki] 2013-02-14 11:37:07 PST
Comment on attachment 713809 [details] [diff] [review]
(mozharness) make the desktop_unittest.py --suite options extend

This can land independently.
http://hg.mozilla.org/build/mozharness/rev/1c8679ffa2be
Not merged to production yet.
Comment 53 Aki Sasaki [:aki] 2013-02-14 17:11:16 PST
(In reply to Aki Sasaki [:aki] from comment #45)
> X  test around a reconfig mid-test

I had an unpatched Rev5 MacOSX Mountain Lion 10.8 mozilla-central debug test crashtest and a Rev3 Fedora 12 mozilla-central debug test mochitest-4 running in staging, and reconfiged to the patched mozharness unittests.

Neither completely blew up.

This isn't a full production-scale test, but the fact that we have this spot testing here with a good result is reassuring.
Comment 54 Aki Sasaki [:aki] 2013-02-14 18:08:33 PST
(In reply to Aki Sasaki [:aki] from comment #45)
> X  test around release- testers (enable in mozilla-{beta,esr*,release} to
> test)

I removed the mozilla-beta MERGE DAY chunk and verified that the new builder_list.py dump changed both the dep/nightly and release- test builders to ScriptFactory.
Comment 55 Aki Sasaki [:aki] 2013-02-14 19:59:13 PST
(In reply to Aki Sasaki [:aki] from comment #45)
> X  test around try

Sendchanged a "real" try build sendchange to my staging master, and it seems to be working fine.

Current status:
I'm going to be on PTO tomorrow (Friday), and would hesitate to land such a large patch right before a long weekend.  I think despite all my best efforts to mitigate risk, I would be surprised if there were absolutely no issues after rolling out.

If wanted, I could potentially roll out on Monday when Canada and the US will be offline.  However, I'm currently planning on letting the merge day happen Tuesday, and when things calm down a bit from that, roll out on maybe Wednesday or Thursday.

I'll most likely have to rebase and un-bitrot my patch after the various merge day patches land, but I've done that enough times to be used to it.


X make UNITTEST_SUITES more like b2g_config
X pass test-masters.sh ?
X PLATFORM_UNITTEST_VARS extra_args
X config_file option per PLATFORM_UNITTEST_VARS
X test-masters.sh
X diff dump_masters
X debug extra_args (download_symbols)
X deal with esr/aurora/beta/release opt/debug unittest suites
X deal with marionette
X  windows only on try
X  not leopard*
X  central inbound try fx-team services-central -- enabled on all m-c level
X    make sure jgriffin knows about this
X deal with ubuntu -- leave alone?
X deal with jetpack -- leave alone?
X test-masters.sh
X diff builder_list
X rebase
X test-masters.sh
X diff builder_list
X patch
X review
X do some spot tests in staging (in parallel)
X  test around try
X  test around release- testers (enable in mozilla-{beta,esr*,release} to test)
X  test around a reconfig mid-test
_ wait til we're ready to take this patch  <-- WE ARE HERE
_ rebase + look at diff
_  diff builder_list if needed
_ test-masters.sh
_ land
_ reconfig
_ verify
_ file merge day bugs
Comment 56 Armen Zambrano [:armenzg] - Engineering productivity 2013-02-15 06:42:14 PST
Merged to mozharness production branch.
Comment 57 Aki Sasaki [:aki] 2013-02-19 18:32:41 PST
Created attachment 715816 [details] [diff] [review]
(configs) fix ubuntu

Fallout from Firefox 21 merging to aurora, but mozharness unittests staying on Firefox 22, I think.
Comment 58 Aki Sasaki [:aki] 2013-02-19 18:33:38 PST
Created attachment 715818 [details] [diff] [review]
(configs) unbitrotted, with ubuntu fix

This is what will land tomorrow, barring any other unforeseen changes.
Comment 59 Aki Sasaki [:aki] 2013-02-20 09:58:54 PST
Comment on attachment 715818 [details] [diff] [review]
(configs) unbitrotted, with ubuntu fix

Carrying forward Ben's and Rail's reviews.
Messed up the commit r= message b/c I'm a dork.
http://hg.mozilla.org/build/buildbot-configs/rev/b4f800452a0a
Comment 60 Aki Sasaki [:aki] 2013-02-20 09:59:23 PST
Comment on attachment 713819 [details] [diff] [review]
(custom) make config refactor a little less ugly, also use deepcopy()

http://hg.mozilla.org/build/buildbotcustom/rev/81b8135751c9
Comment 61 Aki Sasaki [:aki] 2013-02-20 11:49:11 PST
Created attachment 716158 [details] [diff] [review]
stop trying to fetch symbols for b2g tests

The alternative is to turn on symbols in all b2g builds.

This seems less risky atm.
https://tbpl.mozilla.org/php/getParsedLog.php?id=19920245&full=1&branch=mozilla-b2g18
Comment 62 Aki Sasaki [:aki] 2013-02-20 12:06:21 PST
Comment on attachment 716158 [details] [diff] [review]
stop trying to fetch symbols for b2g tests

http://hg.mozilla.org/build/buildbot-configs/rev/8349163e6eaa
Comment 63 Aki Sasaki [:aki] 2013-02-20 13:01:10 PST
Calling this RESO FIXED.
Comment 64 Aki Sasaki [:aki] 2013-02-20 22:06:40 PST
Might as well track bug 843464 for posterity.

Note You need to log in before you can comment on or make changes to this bug.