roll out mozharness unittests to mozilla-central + project branches when ready

RESOLVED FIXED

Status

Release Engineering
Mozharness
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: aki, Assigned: aki)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [mozharness][unittest])

Attachments

(14 attachments, 5 obsolete attachments)

754 bytes, patch
armenzg
: review+
Details | Diff | Splinter Review
38.32 KB, patch
rail
: review+
Details | Diff | Splinter Review
16.76 KB, patch
rail
: review+
Details | Diff | Splinter Review
738 bytes, patch
catlee
: review+
Details | Diff | Splinter Review
1.04 KB, patch
rail
: review+
Details | Diff | Splinter Review
87.33 KB, patch
rail
: review+
Details | Diff | Splinter Review
13.56 KB, patch
jhopkins
: review+
Details | Diff | Splinter Review
1.82 KB, patch
bhearsum
: review+
Details | Diff | Splinter Review
52.64 KB, patch
bhearsum
: review+
Details | Diff | Splinter Review
4.78 KB, patch
bhearsum
: review+
Details | Diff | Splinter Review
123.45 KB, application/x-gzip-compressed
Details
1013 bytes, patch
rail
: review+
Details | Diff | Splinter Review
50.24 KB, patch
aki
: review+
Details | Diff | Splinter Review
862 bytes, patch
rail
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
We should find and fix issues on Cedar first.
(Assignee)

Updated

5 years ago
Depends on: 795525
(Assignee)

Updated

5 years ago
Depends on: 795533
(Assignee)

Updated

5 years ago
Depends on: 795535
(Assignee)

Updated

5 years ago
Depends on: 802796
(Assignee)

Updated

5 years ago
Depends on: 805933
(Assignee)

Updated

5 years ago
Depends on: 806524
(Assignee)

Comment 1

5 years ago
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?
(Assignee)

Comment 2

5 years ago
Let's also get the DesktopUnittestOutputParser patch from bug 774535 reviewed+landed first.
(Assignee)

Updated

5 years ago
Depends on: 806651
(Assignee)

Comment 3

5 years ago
It would be nice to solve bug 807094 and bug 758975 before this rolls out, but it's not necessarily a hard block.
(Assignee)

Comment 4

5 years ago
I suppose bug 793642 is what bug 758975 is duped to.
(Assignee)

Updated

5 years ago
Depends on: 807094
(Assignee)

Updated

5 years ago
Depends on: 793642
(Assignee)

Updated

5 years ago
Depends on: 808814
(Assignee)

Comment 5

5 years ago
I'm going to start the ball rolling to get these unittests live this week -- any objections?
(Assignee)

Comment 6

5 years ago
Created attachment 680929 [details] [diff] [review]
enable mozharness unittests on ash (aurora mirror)
Attachment #680929 - Flags: review?(armenzg)

Updated

5 years ago
Depends on: 808536

Updated

5 years ago
Attachment #680929 - Flags: review?(armenzg) → review+
(Assignee)

Comment 7

5 years ago
Comment on attachment 680929 [details] [diff] [review]
enable mozharness unittests on ash (aurora mirror)

http://hg.mozilla.org/build/buildbot-configs/rev/c50d11fe5ce9
Attachment #680929 - Flags: checked-in+
(Assignee)

Updated

5 years ago
No longer depends on: 793642
(Assignee)

Updated

5 years ago
Depends on: 811453
(Assignee)

Updated

5 years ago
No longer depends on: 811453
(Assignee)

Comment 8

5 years ago
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.
Depends on: 812723
Depends on: 812726
Depends on: 812727
(Assignee)

Updated

5 years ago
Depends on: 821445
(Assignee)

Updated

5 years ago
Depends on: 826472
(Assignee)

Comment 9

5 years ago
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.
Attachment #707161 - Flags: review?(rail)
Attachment #707161 - Flags: review?(rail) → review+
(Assignee)

Comment 10

5 years ago
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
Attachment #707161 - Flags: checked-in+
(Assignee)

Comment 11

5 years ago
Created attachment 708850 [details] [diff] [review]
(tools) production-masters.json limit_mobile_platforms

Needs the corresponding configs (part 2) patch.
(Assignee)

Comment 12

5 years ago
Created attachment 708935 [details] [diff] [review]
(braindump) allow for -j in dump_masters.sh

-8 is deprecated as well.
Attachment #708935 - Flags: review?(catlee)
(Assignee)

Comment 13

5 years ago
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.
(Assignee)

Comment 14

5 years ago
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

Updated

5 years ago
Attachment #708935 - Flags: review?(catlee) → review+
(Assignee)

Comment 15

5 years ago
Comment on attachment 708935 [details] [diff] [review]
(braindump) allow for -j in dump_masters.sh

http://hg.mozilla.org/build/braindump/rev/1b303076edf3
Attachment #708935 - Flags: checked-in+
(Assignee)

Updated

4 years ago
Depends on: 837695
(Assignee)

Comment 16

4 years ago
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.
(Assignee)

Updated

4 years ago
Depends on: 838485
(Assignee)

Comment 17

4 years ago
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.
Attachment #711058 - Flags: review?(rail)
(Assignee)

Updated

4 years ago
Attachment #708961 - Flags: review?(rail)
(Assignee)

Updated

4 years ago
Attachment #708850 - Flags: review?(rail)
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.
Attachment #711058 - Flags: review?(rail) → review-
(Assignee)

Comment 19

4 years ago
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.
Attachment #711598 - Flags: review?(rail)
(Assignee)

Comment 20

4 years ago
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.
Attachment #711598 - Attachment is obsolete: true
Attachment #711598 - Flags: review?(rail)
(Assignee)

Comment 21

4 years ago
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.
Attachment #711058 - Attachment is obsolete: true
Attachment #711604 - Flags: review?(rail)
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
Attachment #711604 - Flags: review?(rail) → review-
Created attachment 711861 [details] [diff] [review]
(configs) some fixes
Attachment #708961 - Flags: review?(rail) → review+
Attachment #708850 - Flags: review?(rail) → review+
(Assignee)

Comment 24

4 years ago
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.
Attachment #711604 - Attachment is obsolete: true
Attachment #711915 - Flags: review?(rail)
Comment on attachment 711915 [details] [diff] [review]
(configs) part 2, take 4: split out mozilla-tests/mobile_config.py

woot! the interdiff lgtm!
Attachment #711915 - Flags: review?(rail) → review+
Attachment #711861 - Attachment is obsolete: true
(Assignee)

Comment 26

4 years ago
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
Attachment #711915 - Flags: checked-in+
(Assignee)

Comment 27

4 years ago
Comment on attachment 708850 [details] [diff] [review]
(tools) production-masters.json limit_mobile_platforms

http://hg.mozilla.org/build/tools/rev/14c1e7b65d88
Attachment #708850 - Flags: checked-in+
(Assignee)

Comment 28

4 years ago
Comment on attachment 708961 [details] [diff] [review]
(custom) remove enable_mobile_unittests support

http://hg.mozilla.org/build/buildbotcustom/rev/43d91fd84700
Attachment #708961 - Flags: checked-in+
(Assignee)

Updated

4 years ago
Depends on: 839707
(Assignee)

Comment 29

4 years ago
Working on the 3rd and final set of patches.
Progress here:
https://github.com/escapewindow/build-buildbot-configs/compare/master...mhunit
Status: NEW → ASSIGNED
(Assignee)

Comment 30

4 years ago
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.
Flags: needinfo?(marco.zehe)
(Assignee)

Comment 31

4 years ago
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.
Attachment #712067 - Flags: review?(jhopkins)
(Assignee)

Updated

4 years ago
Depends on: 838325
(Assignee)

Updated

4 years ago
Depends on: 838492
(Assignee)

Comment 32

4 years ago
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
(Assignee)

Updated

4 years ago
Depends on: 840202
(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.
Flags: needinfo?(marco.zehe)
in production
(Assignee)

Updated

4 years ago
Depends on: 840305
(Assignee)

Updated

4 years ago
Depends on: 840309
(Assignee)

Comment 35

4 years ago
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.
Depends on: 837022
Attachment #712067 - Flags: review?(jhopkins) → review+
(Assignee)

Comment 36

4 years ago
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
Attachment #712067 - Flags: checked-in+
(Assignee)

Updated

4 years ago
No longer depends on: 840305
(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. :)
(Assignee)

Comment 38

4 years ago
The thunderbird config cleanup (configs part 2.5) is live.  Still working on the 3rd and potentially final config patch.
(Assignee)

Comment 39

4 years ago
: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
Flags: needinfo?(bhearsum)
(Assignee)

Comment 40

4 years ago
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
Depends on: 840694
No longer depends on: 837022
(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.
Flags: needinfo?(bhearsum)
(Assignee)

Comment 42

4 years ago
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
Attachment #713809 - Flags: review?(bhearsum)
(Assignee)

Comment 43

4 years ago
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.
Attachment #713812 - Flags: review?(bhearsum)
(Assignee)

Comment 44

4 years ago
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.
Attachment #713814 - Flags: review?(bhearsum)
(Assignee)

Comment 45

4 years ago
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
(Assignee)

Comment 46

4 years ago
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.
Attachment #713812 - Attachment is obsolete: true
Attachment #713812 - Flags: review?(bhearsum)
Attachment #713819 - Flags: review?(bhearsum)
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?
Attachment #713809 - Flags: review?(bhearsum) → review+
(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?
Attachment #713819 - Flags: review?(bhearsum) → review+
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 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.
Attachment #713814 - Flags: review?(bhearsum) → review+
(Assignee)

Comment 51

4 years ago
(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.
(Assignee)

Updated

4 years ago
Attachment #713952 - Attachment mime type: text/plain → application/x-gzip-compressed
(Assignee)

Comment 52

4 years ago
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.
Attachment #713809 - Flags: checked-in+
(Assignee)

Comment 53

4 years ago
(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.
(Assignee)

Comment 54

4 years ago
(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.
(Assignee)

Comment 55

4 years ago
(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

4 years ago
Merged to mozharness production branch.
(Assignee)

Comment 57

4 years ago
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.
Attachment #715816 - Flags: review?(rail)
(Assignee)

Comment 58

4 years ago
Created attachment 715818 [details] [diff] [review]
(configs) unbitrotted, with ubuntu fix

This is what will land tomorrow, barring any other unforeseen changes.
Attachment #715816 - Flags: review?(rail) → review+
(Assignee)

Comment 59

4 years ago
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
Attachment #715818 - Flags: review+
Attachment #715818 - Flags: checked-in+
(Assignee)

Comment 60

4 years ago
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
Attachment #713819 - Flags: checked-in+
(Assignee)

Comment 61

4 years ago
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
Attachment #716158 - Flags: review?(rail)
Attachment #716158 - Flags: review?(rail) → review+
(Assignee)

Comment 62

4 years ago
Comment on attachment 716158 [details] [diff] [review]
stop trying to fetch symbols for b2g tests

http://hg.mozilla.org/build/buildbot-configs/rev/8349163e6eaa
Attachment #716158 - Flags: checked-in+
(Assignee)

Comment 63

4 years ago
Calling this RESO FIXED.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Depends on: 843467
(Assignee)

Comment 64

4 years ago
Might as well track bug 843464 for posterity.
Depends on: 843464
Blocks: 843950
Product: mozilla.org → Release Engineering

Updated

3 years ago
Component: General Automation → Mozharness
You need to log in before you can comment on or make changes to this bug.