Closed Bug 1384905 Opened 2 years ago Closed 2 years ago

[tc-migration] Windows 8 should not be triggered on try (unless specified)

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla56

People

(Reporter: jlorenzo, Assigned: jlorenzo)

References

Details

Attachments

(2 files)

Fallout from yesterday's taskcluster migration. The win8 queue is overgrowing because we're now trigger jobs by default on xpcshell.
Task graph *WITHOUT* specifiying "Windows 8": https://tools.taskcluster.net/groups/J45992bqQHWe6DIlXDXtmA
Its try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3d4c78a73ec2f3da0460579bc415f892f19edbc8
Results => OK, only windows 10 (pure taskcluster) is scheduled

Task graph *WITH* specifiying "Windows 8": https://tools.taskcluster.net/groups/LZkbCesHShO4oos72E9RUg
Its try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=687b3439c3fc3b368a43c581ac1aa39d91a4f602
Results => OK, only windows 8 is scheduled, no windows 10.
Comment on attachment 8890824 [details]
Bug 1384905 - [tc-migration] Windows 8 should not be triggered on try (unless specified)

https://reviewboard.mozilla.org/r/162046/#review167328

::: taskcluster/taskgraph/try_option_syntax.py:132
(Diff revision 4)
>      # '10.6': [..TODO..],
>      # '10.8': [..TODO..],
>      # 'Android 2.3 API9': [..TODO..],
>      # 'Windows 7':  [..TODO..],
>      # 'Windows 7 VM': [..TODO..],
> -    # 'Windows 8':  [..TODO..],
> +    'Windows 8':  ['windows8-64'],

Can you do 'Windows 10' and 'Windows 7' while you're here

::: taskcluster/taskgraph/try_option_syntax.py:583
(Diff revision 4)
> -            return True
> +            if 'platforms' in test:
> +                platform = attr('test_platform', '').split('/')[0]
> +                # Platforms can be forced by syntax like "-u xpcshell[Windows 8]"
> +                return platform in test['platforms']
> +            else:
> +                return check_run_on_projects()

unlike what I expected, looks like this is indeed needed here (we had support for not-try-by-default for builds, but not tests I guess.)

::: taskcluster/ci/test/tests.yml:76
(Diff revision 5)
>      docker-image: {"in-tree": "desktop1604-test"}
>      run-on-projects:
>          by-test-platform:
> +            # Deactivate try on Buildbot, by default. We don't have enough machines
> +            windows8-64.*: ['mozilla-release', 'mozilla-beta', 'mozilla-central', 'mozilla-inbound', 'autoland']
> +            windows8-64-devedition: built-projects  # No dev edition outside of supported branches

throughout, we need `windows8-64-devedition/opt`
Attachment #8890824 - Flags: review?(bugspam.Callek) → review+
Pushed by jlorenzo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/dcd146cf33b9
[tc-migration] Windows 8 should not be triggered on try (unless specified) r=Callek
https://hg.mozilla.org/mozilla-central/rev/dcd146cf33b9
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Attached patch Take 2Splinter Review
The first version of the patch didn't account for the fact that if we did -p all we still couldn't do -u foo[Windows 8]

specifically: -p all means self.platforms is None
https://hg.mozilla.org/mozilla-central/file/tip/taskcluster/taskgraph/try_option_syntax.py#l563

Then we check if the *test* has a run_on_projects that has either `all` or `try`, it doesn't so we abort.

In new code, we merely set a flag to say don't-run-me-by-default and use that in a few places.
Attachment #8891453 - Flags: review?(dustin)
Attachment #8891453 - Flags: feedback?(jlorenzo)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 8891453 [details] [diff] [review]
Take 2

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

So basically this is an issue with test platforms vs. build platforms.. yuck.
Attachment #8891453 - Flags: review?(dustin) → review+
Pushed by Callek@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bd0071cbb121
Take 2 - [tc-migration] Windows 8 should not be triggered on try (unless specified) r=dustin
https://hg.mozilla.org/mozilla-central/rev/bd0071cbb121
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Comment on attachment 8891453 [details] [diff] [review]
Take 2

I gave positive feedback on IRC on Friday, but forgot to port it here. LGTM.
Attachment #8891453 - Flags: feedback?(jlorenzo) → feedback+
Duplicate of this bug: 1374860
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.