Closed Bug 1344321 Opened 3 years ago Closed 3 years ago

enable nightly linux tests on tc

Categories

(Release Engineering :: Release Automation: Other, enhancement, P1)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aki, Assigned: aki)

References

Details

Attachments

(8 files, 6 obsolete files)

4.70 KB, patch
Callek
: review+
Details | Diff | Splinter Review
203.58 KB, image/png
Details
59 bytes, text/x-review-board-request
dustin
: review+
Details
59 bytes, text/x-review-board-request
dustin
: review+
Details
47 bytes, text/x-github-pull-request
emorley
: review+
Details | Review
8.32 KB, patch
Details | Diff | Splinter Review
2.03 KB, patch
dustin
: review+
Details | Diff | Splinter Review
10.66 KB, patch
dustin
: review+
Details | Diff | Splinter Review
We have ci test coverage on m-c and m-a, but we won't on m-b (and don't on jamun).
Attachment #8843395 - Flags: review?(bugspam.Callek)
Comment on attachment 8843395 [details] [diff] [review]
enable-linux-tests.diff

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

Overall looks good. Can you check the piece I mentioned though before landing, lmk if you need more guidance

::: taskcluster/taskgraph/target_tasks.py
@@ +149,5 @@
>          if platform in ('android-api-15', 'android-x86'):
>              return True
>          if platform in ('linux64-nightly', 'linux-nightly'):
> +            if task.kind in ["test"]:
> +                return True

I'd love a list of added tasks when run with target specified here, since I fear this may trigger builds for non-nightly based linux... (due to non-nightly linux tests needing it)
Attachment #8843395 - Flags: review?(bugspam.Callek) → review+
(In reply to Justin Wood (:Callek) from comment #1)
> ::: taskcluster/taskgraph/target_tasks.py
> @@ +149,5 @@
> >          if platform in ('android-api-15', 'android-x86'):
> >              return True
> >          if platform in ('linux64-nightly', 'linux-nightly'):
> > +            if task.kind in ["test"]:
> > +                return True
> 
> I'd love a list of added tasks when run with target specified here, since I
> fear this may trigger builds for non-nightly based linux... (due to
> non-nightly linux tests needing it)

https://tools.taskcluster.net/task-group-inspector/#/fwdKjG8jR1eZgGBnHXjHLA?_k=ozgwuy
Attached patch add-nightly-tests-m-c.diff (obsolete) — Splinter Review
This is what m-c's patch will look like.
I'll have to massage this to uplift to aurora; it'll look somewhere between this and the first patch you reviewed.  I can send the final m-a patch for r? if you'd like.
Attachment #8843423 - Flags: review?(bugspam.Callek)
Attachment #8843423 - Flags: review?(bugspam.Callek) → review+
So we're fine on jamun or m-b.  On m-c or m-a, this turns on nightly builds on push :(
Worst case, we need to land https://bug1344321.bmoattachments.org/attachment.cgi?id=8843395 on m-b after merge uplift =\

We still need a long-term fix here.
CCing jlund+jlorenzo for comment 7.  Without that patch, I believe we won't have linux tests on m-b.
I think the real fix here is a new nightly-test kind.  This seems doable, but requires more headspace+energy than I have this week.
Priority: -- → P1
Keywords: leave-open
Dustin, halp! :)

- We're currently missing all nightly tests for tc nightly builds.

- This commit [1] enables nightly tests, but looks like it also enables nightly builds+tests per push on all branches, which is not what we want.

- I thought it might be best to create a nightly-test kind, kind of like nightly-l10n or nightly-l10n-signing.  However, that makes the test-platforms.yml logic a little unhappy [2].

    - I thought about softlinking test-platforms.yml, test-sets.yml, and tests.yml from taskcluster/ci/test/ to taskcluster/ci/nightly-test/, and then hacking the platforms to allow for -nightly, and decided it might be best to ask what the best approach might be, before I do something ugly that we'll all regret later.

Any ideas? :)

[1] https://hg.mozilla.org/integration/mozilla-inbound/rev/a557cf5a3330bdd4f0d782240a09a4c92d4b7a2e
[2] https://hg.mozilla.org/integration/mozilla-inbound/file/295f8f82ced3/taskcluster/taskgraph/task/test.py#l33
Flags: needinfo?(dustin)
18:34 <•jlund> what if you add `run-on-projects: [ ]`
18:34 <•jlund> under each of those *-nightly stanzas
18:34 <•jlund> I thinkt hat would remove them from per-push
18:35 <•jlund> then assuming you key off something like: https://hg.mozilla.org/integration/mozilla-inbound/rev/a557cf5a3330bdd4f0d782240a09a4c92d4b7a2e#l3.37
18:35 <•jlund> when you trigger a hook, you should be good
18:37 <•jlund> I'm a bit muddled on whether your issues is for nightlies, CI on nightly repos, betas, or CI on beta repo
18:37 <•jlund> so I'll step back unless you want to meet and have a second pair of eyes
18:37 <•jlund> issue*

I'm going to try this.
18:53 <Callek> aki: jlund that would sort of work,  currently no support in the test sets now,  would need it in https://hg.mozilla.org/integration/mozilla-inbound/file/a557cf5a3330/taskcluster/ci/test/tests.yml .... which assumes every test adds that (our they too get nightlies)
18:53 <Callek> So we'd likely need support added to test sets as well. ...

When we add the above `run-on-projects: [ ]` to 

    run-on-projects:
        by-test-platform:
            windows.*: ['mozilla-central', 'try']
            linux64-qr/.*: ['graphics']
            default: ['all']

Do we add `*-nightly/.*: [ ]` to every run-on-projects: by-test-platform: ?

http://gecko.readthedocs.io/en/latest/taskcluster/taskcluster/kinds.html#tests says "This process generates all test jobs, regardless of tree or try syntax. It is up to a later stage of the task-graph generation (the target set) to select the tests that will actually be performed."  Should we have `target_task_method default` filter out nightly tests and builds?  What effect does that have on try?
This prevents us from testing nightlies on try in the standard try syntax, but if we tool-ify https://gist.github.com/acmiyaguchi/f20c4f47e723b3a94ad9a3f82da457f9 , then developers can test nightlies-on-try as well.

What do you think?
Attachment #8843423 - Attachment is obsolete: true
Attachment #8844320 - Flags: review?(dustin)
(In reply to Aki Sasaki [:aki] from comment #10)
> Dustin, halp! :)
> 
> - We're currently missing all nightly tests for tc nightly builds.
> 
> - This commit [1] enables nightly tests, but looks like it also enables
> nightly builds+tests per push on all branches, which is not what we want.

The way nightlies are structured right now, they're basically a different platform variant from opt, debug, asan, etc.  This makes a lot of sense if you want to allow a try push to generate a nightly-style build.  I don't know what the try syntax would be (-p linux64-nightly?), but its effect would be to say "include the linux64-nightly build job in the target task set".

The alternative would be to make the build jobs behave differently when run in a nightly context (that is, from the cron task).  In this scenario, the linux64-opt build would be "nightly-style" when params['nightly'] is true.  All of the tests that normally run on linux64-opt would run there, too.

It seems the first option is preferred, and anyway is how things stand now.  Adding tests for these nightly platforms means that, if the tests are selected (and most projects select tests on the basis of the run_on_projects attribute), then the builds that they depend on will be selected, too.   That is, I think, what you're seeing here.  The fix is, don't select the tests based on the nightly builds by default in try.

> - I thought it might be best to create a nightly-test kind, kind of like
> nightly-l10n or nightly-l10n-signing.  However, that makes the
> test-platforms.yml logic a little unhappy [2].
> 
>     - I thought about softlinking test-platforms.yml, test-sets.yml, and
> tests.yml from taskcluster/ci/test/ to taskcluster/ci/nightly-test/, and
> then hacking the platforms to allow for -nightly, and decided it might be
> best to ask what the best approach might be, before I do something ugly that
> we'll all regret later.

I don't think this is necessary.  These are fundamentally just tests, so they don't need a separate kind.  I think attachment 8844320 [details] [diff] [review] is in the right direction: just don't include this platform in the target task set except where desired.  It occurs to me -- does it make sense to include opt and/or debug in mozilla-beta and mozilla-release pushes?
Flags: needinfo?(dustin)
Comment on attachment 8844320 [details] [diff] [review]
enable nightly tests; disable nightly builds+tests in default target_task_method

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

I think this is the right direction!

::: taskcluster/taskgraph/target_tasks.py
@@ +34,2 @@
>  @_target_task('try_option_syntax')
>  def target_tasks_try_option_syntax(full_task_graph, parameters):

Does this do what you want with try jobs, with no changes?

@@ +80,5 @@
>      return target_tasks_labels
>  
>  
>  @_target_task('default')
> +def target_tasks_default(full_task_graph, parameters, enable_nightly=False):

Let's make `include_nightly` a parameter that can be set or un-set per branch in https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/decision.py#35.  I think it should apply for mozilla-release, too, right?

@@ +223,5 @@
>              if task.kind not in [
>                  'balrog', 'beetmover', 'beetmover-checksums', 'beetmover-l10n',
>                  'checksums-signing', 'nightly-l10n', 'nightly-l10n-signing'
>              ]:
>                  return task.attributes.get('nightly', False)

Can this clause go away now?  I don't really understand what it's trying to do..

::: taskcluster/taskgraph/transforms/tests.py
@@ +586,5 @@
>  @transforms.add
> +def set_nightly(config, tests):
> +    """Set nightly attribute for tests."""
> +    for test in tests:
> +        if test['test-platform'] in ['linux32-nightly/opt', 'linux64-nightly/opt']:

This attribute is already set for builds:
  https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/build/linux.yml#150
so I think it might make more sense to carry this attribute forward automatically to the tests, in
  https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/task/test.py#30

This would avoid listing platforms here (for example, we'll probably forget win64-nightly/opt until we notice it's building on try).

@@ +589,5 @@
> +    for test in tests:
> +        if test['test-platform'] in ['linux32-nightly/opt', 'linux64-nightly/opt']:
> +            attributes = test.get('attributes', {})
> +            attributes['nightly'] = True
> +            test['attributes'] = attributes

test.setdefault('attributes', {})['nightly'] = True
Attachment #8844320 - Flags: review?(dustin) → review-
Some observations from Beta:

* We run Linux32 Nightly builds that show up where you'd expect them in the Treeherder view on the Linux opt line (in line with how they have historically with buildbot-scheduled jobs as well). However, the resulting tests appear way at the bottom of the list on linuxXX-nightly opt lines with no immediately-obvious connection to those builds in a way different place. This applies to both Linux32 and Linux64.
* We're also running Linux64 PGO builds and tests on each push, which seems redundant with the aforementioned Nightly builds/tests that are also running, assuming they're also PGOed.
Attached image linuxplatforms.png
Something like https://github.com/mozilla/treeherder/compare/master...KWierso:linuxnightly?expand=1 would at least get them listed closer to the expected locations. (See attached)

I tried dropping the " Nightly" from the pretty names so they match up with the existing "Linux (x64) opt" lines, but Treeherder isn't smart enough to coalesce them, and just adds a somewhat confusing duplicated platform line with the new jobs on it.
(In reply to Wes Kocher (:KWierso) from comment #17)
> Created attachment 8845132 [details]
> linuxplatforms.png
> 
> Something like
> https://github.com/mozilla/treeherder/compare/master...KWierso:
> linuxnightly?expand=1 would at least get them listed closer to the expected
> locations. (See attached)
> 
> I tried dropping the " Nightly" from the pretty names so they match up with
> the existing "Linux (x64) opt" lines, but Treeherder isn't smart enough to
> coalesce them, and just adds a somewhat confusing duplicated platform line
> with the new jobs on it.

Sure.  Will this approach work for you RyanVM?
Flags: needinfo?(ryanvm)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #16)
> * We run Linux32 Nightly builds that show up where you'd expect them in the
> Treeherder view on the Linux opt line (in line with how they have
> historically with buildbot-scheduled jobs as well). However, the resulting
> tests appear way at the bottom of the list on linuxXX-nightly opt lines with
> no immediately-obvious connection to those builds in a way different place.
> This applies to both Linux32 and Linux64.

I think bug 1286086 would go some way to sorting this out -- we cram a lot into the "platform" right now, without much structure.  That bug is stalled, unfortunately.

> * We're also running Linux64 PGO builds and tests on each push, which seems
> redundant with the aforementioned Nightly builds/tests that are also
> running, assuming they're also PGOed.

This can be fixed with a more refined filter for the branch.  Maybe in a second patch :)
Comment on attachment 8845264 [details]
bug 1344321 - add nightly test support.  a=release

https://reviewboard.mozilla.org/r/118436/#review120490

::: commit-message-9dd78:5
(Diff revision 1)
> +bug 1344321 - add nightly test support. r=dustin a=release
> +
> +this patch:
> +
> +- adds linux{32,64}-nightly/opt test platformss that mirror the non-nightly test platforms.

platformsssss :)

::: taskcluster/taskgraph/try_option_syntax.py:296
(Diff revision 1)
>          self.talos_trigger_tests = 0
>          self.env = []
>          self.profile = False
>          self.tag = None
>          self.no_retry = False
> +        self.include_nightly = include_nightly

It looks like it would be just as easy to have try ignore the parameter and add an `--include-nightly` option to try syntax that will set this value.  That would make try runs a lot easier!
Attachment #8845264 - Flags: review?(dustin) → review+
(In reply to Dustin J. Mitchell [:dustin] from comment #21)

> > * We're also running Linux64 PGO builds and tests on each push, which seems
> > redundant with the aforementioned Nightly builds/tests that are also
> > running, assuming they're also PGOed.
> 
> This can be fixed with a more refined filter for the branch.  Maybe in a
> second patch :)

There is a second patch, I just flagged RyanVM for r? :)
(In reply to Dustin J. Mitchell [:dustin] from comment #22)
> ::: taskcluster/taskgraph/try_option_syntax.py:296
> (Diff revision 1)
> >          self.talos_trigger_tests = 0
> >          self.env = []
> >          self.profile = False
> >          self.tag = None
> >          self.no_retry = False
> > +        self.include_nightly = include_nightly
> 
> It looks like it would be just as easy to have try ignore the parameter and
> add an `--include-nightly` option to try syntax that will set this value. 
> That would make try runs a lot easier!

You're right! New patches submitted...
(In reply to Aki Sasaki [:aki] from comment #18)
> Sure.  Will this approach work for you RyanVM?

Ideally we'd get the Nightly builds on the same line with their tests, but this is better than the status quo anyway.
Flags: needinfo?(ryanvm)
Hm, jamun isn't running nightly signing.  I need to double check things.
Comment on attachment 8845265 [details]
bug 1344321 - remove linux64-pgo from target_tasks_mozilla_beta.  a=release

https://reviewboard.mozilla.org/r/118438/#review120764
Attachment #8845265 - Flags: review?(dustin) → review+
Comment on attachment 8845264 [details]
bug 1344321 - add nightly test support.  a=release

Re-requesting review, after addressing review comments and re-adding linux nightly signing back to mozilla-beta.
Attachment #8845264 - Flags: review+ → review?(dustin)
Attached file fix treeherder linux-nightly (obsolete) —
Attachment #8845689 - Flags: review?(ryanvm)
I noticed tonight that we're not running SM jobs on Beta anymore either. Thankfully there's still a couple buildbot-based Windows SM jobs hanging around, but we'll need to fix that too.
Comment on attachment 8845264 [details]
bug 1344321 - add nightly test support.  a=release

https://reviewboard.mozilla.org/r/118436/#review120794

::: taskcluster/taskgraph/target_tasks.py
(Diff revisions 1 - 3)
>      of linux, plus android CI. The candidates build process involves a pipeline
>      of builds and signing, but does not include beetmover or balrog jobs."""
>  
>      def filter(task):
> -        if not filter_for_project(task, parameters):
> -            return False

did you mean to remove this?
Attachment #8845264 - Flags: review?(dustin) → review+
(In reply to Dustin J. Mitchell [:dustin] from comment #38)
> Comment on attachment 8845264 [details]
> bug 1344321 - add nightly test support.  a=release
> 
> https://reviewboard.mozilla.org/r/118436/#review120794
> 
> ::: taskcluster/taskgraph/target_tasks.py
> (Diff revisions 1 - 3)
> >      of linux, plus android CI. The candidates build process involves a pipeline
> >      of builds and signing, but does not include beetmover or balrog jobs."""
> >  
> >      def filter(task):
> > -        if not filter_for_project(task, parameters):
> > -            return False
> 
> did you mean to remove this?

Yeah, that's what removed signing =\
Pushed by asasaki@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1bce56d3f45d
add nightly test support. r=dustin a=release
https://hg.mozilla.org/integration/autoland/rev/96e3bc2098c8
remove linux64-pgo from target_tasks_mozilla_beta. r=dustin a=release
status:

_ uplift above 2 patches to m-c
_ uplift above 2 patches to m-a
_ enable sm jobs on m-b - write patch
  _ get review
  _ land sm jobs patch on m-b
  _ land sm jobs patch on m-i
  _ land sm jobs patch on m-a
_ review for treeherder patch
  _ merge treeherder patch
  _ roll out treeherder patch?
:sfink, I'm trying to include sm jobs in my target_task_method https://hg.mozilla.org/integration/autoland/file/1bce56d3f45d/taskcluster/taskgraph/target_tasks.py#l211

None of these things seem to work:

         if task.kind in ["spidermonkey"]:

or

         platform = task.attributes.get('build_platform')
         if platform in ('android-api-15', 'android-x86', 'linux64-asan',
                        'linux64-add-on-devel'):
                        'linux64-add-on-devel', 'linux64-haz', 'linux64-shell-haz',
                        'sm-arm-sim', 'sm-arm64-sim', 'sm-asan',
                        'sm-compacting', 'sm-mozjs-sys', 'sm-msan',
                        'sm-nonunified', 'sm-package', 'sm-plain',
                        'sm-plain', 'sm-rootanalysis', 'sm-tsan'):

(I am adding the -haz jobs successfully, though); or even

        if task.label.startswith("sm-"):

Do you have an idea of what I should be doing?
Flags: needinfo?(sphink)
Maybe I should be explicitly disabling rather than explicitly enabling.
Attachment #8845689 - Attachment is obsolete: true
Attachment #8845689 - Flags: review?(ryanvm)
Attachment #8845626 - Flags: review+
status:

X uplift above 2 patches to m-c
_ uplift above 2 patches to m-a
_ enable sm jobs on m-b - figure out how; write patch
  _ get review
  _ land sm jobs patch on m-b
  _ land sm jobs patch on m-i
  _ land sm jobs patch on m-a
X review for treeherder patch
  X merge treeherder patch
  ? roll out treeherder patch?
(In reply to Aki Sasaki [:aki] from comment #42)
> :sfink, I'm trying to include sm jobs in my target_task_method
> https://hg.mozilla.org/integration/autoland/file/1bce56d3f45d/taskcluster/
> taskgraph/target_tasks.py#l211
> 
> None of these things seem to work:
> 
>          if task.kind in ["spidermonkey"]:
> 
> Do you have an idea of what I should be doing?

First, no. I've never looked at that portion of the pipeline before.

But I tried running some taskgraph commands, and it really seems like either task.kind or task.attributes.get('kind') should work. Both worked for me testing locally. If that isn't working, it makes me suspect that the tasks aren't making it here. Is that possible? Does the platform mismatch or something? (No clue how that works.)

I didn't apply your patch, I just used your parameters.yml with the nightly stuff commented out.
Flags: needinfo?(sphink)
Aha! target_task_method mozilla_beta_tasks *keeps* spidermonkey tasks, at least as it exists on my laptop atm.
Something else is filtering it out later.  Still digging.
Attached patch wip.diffSplinter Review
Attachment #8844320 - Attachment is obsolete: true
Spidermonkey tasks are getting optimized out.  I'm not sure if that's because the revisions I've been specifying in my parameters file have been non-spidermonkey-related.  I'm also wondering if I should turn optimization off for beta+release.
Could it be the 'when' clause? They're all discarded if you don't change stuff in one of the right directories.
Try push with optimization on has sm tasks: https://tools.taskcluster.net/task-group-inspector/#/YQX7_FLpTAuvm1LrsaJPvA/KDqXuwH1S3uyNcQv522C-w?_k=t47b5v

I'm rebasing to see if the build bustage is related to the changeset I'm based off of on inbound.
https://tools.taskcluster.net/task-group-inspector/#/bJx5Y2BrRzSVMRYOX8Z7jQ?_k=569fij is looking good, with spidermonkey jobs on a mozilla_beta_tasks target_task_method run.

I'm:

- making sure the above set of patches don't cause any unexpected behavior, esp try nightlies on non- --include-nightly pushes
- cleaning up the patch
- trying to see if the patch applies to m-b, and if not, what needs to change or be uplifted

while I wait for the results of the try push.
I'm a little concerned with all the windows jobs, but if we have to we can kill those in the target_task_method.  I'd have guessed run-on-projects would have done that already.
(In reply to Aki Sasaki [:aki] from comment #56)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=4667f6fdb339

This is a push from the refactored mozilla-beta set of patches... largely the same as the m-i version, but changed enough to need tlc to land.
We may want to disable:

- artifact builds
- windows and mac builds

and associated tests on beta.

Ideally we do this via run-on-projects, but this has dragged out to the point that I want to just kill them in mozilla_beta_tasks and have done with this bug.
Attached patch nightly-tests3.diff (obsolete) — Splinter Review
This patch finishes adding the `include_nightly` parameter, and switches `mozilla_beta_tasks` from off-by-default, enable-specific to on-by-default, disable-specific.

Afterwards, we end up with spidermonkey tasks.
Attachment #8846925 - Flags: review?(dustin)
Attached patch disable windows, osx, artifact (obsolete) — Splinter Review
This patch disables windows, mac, and artifact builds from mozilla_beta_tasks.
Attachment #8846926 - Flags: review?(dustin)
Attachment #8846926 - Attachment is obsolete: true
Attachment #8846926 - Flags: review?(dustin)
Attachment #8846933 - Flags: review?(dustin)
Status:

X uplift above 2 patches to m-c
_ uplift above 2 patches to m-a
X enable sm jobs on m-b - figure out how; write patches
  _ get review
  _ land sm jobs patches on m-b
  _ land sm jobs patches on m-i
  _ land sm jobs patches on m-a
X review for treeherder patch
  X merge treeherder patch
  ? roll out treeherder patch?
:Callek says we need artifact builds on beta.
Attachment #8846933 - Attachment is obsolete: true
Attachment #8846933 - Flags: review?(dustin)
Attachment #8847119 - Flags: review?(dustin)
Comment on attachment 8846925 [details] [diff] [review]
nightly-tests3.diff

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

I'm not totally sure what this is intending to accomplish - if the spidermonkey tasks were, indeed, being optimized then this change to target task methods shouldn't make a difference.  That said, you could set `optimized_target_tasks` to false for the mozilla-beta project to avoid that.

I'm also confused why include_nightly is set to True for try..

::: taskcluster/taskgraph/decision.py
@@ +38,5 @@
>          # Always perform optimization.  This makes it difficult to use try
>          # pushes to run a task that would otherwise be optimized, but is a
>          # compromise to avoid essentially disabling optimization in try.
>          'optimize_target_tasks': True,
> +        'include_nightly': True,

If this is true by default for try, do you need `--no-include-nightly` in try syntax?
Attachment #8846925 - Flags: review?(dustin)
Attachment #8847119 - Flags: review?(dustin) → review+
(In reply to Dustin J. Mitchell [:dustin] from comment #66)
> Comment on attachment 8846925 [details] [diff] [review]
> nightly-tests3.diff
> 
> Review of attachment 8846925 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm not totally sure what this is intending to accomplish - if the
> spidermonkey tasks were, indeed, being optimized then this change to target
> task methods shouldn't make a difference.  That said, you could set
> `optimized_target_tasks` to false for the mozilla-beta project to avoid that.

I wasn't sure if we want to avoid optimizing on beta or not (comment 50).  I could still set it to false, but it appears we're getting spidermonkey runs (see latest 4 try runs).

> I'm also confused why include_nightly is set to True for try..

This is specifically so when I run a mozilla_beta_tasks target_task_method decision task on try, I don't also have to tweak the include_nightly flag.  This doesn't affect the try_option_syntax target_task_method.

> ::: taskcluster/taskgraph/decision.py
> @@ +38,5 @@
> >          # Always perform optimization.  This makes it difficult to use try
> >          # pushes to run a task that would otherwise be optimized, but is a
> >          # compromise to avoid essentially disabling optimization in try.
> >          'optimize_target_tasks': True,
> > +        'include_nightly': True,
> 
> If this is true by default for try, do you need `--no-include-nightly` in
> try syntax?

Looks like no... all my try-params.yml optimization runs don't have nightly, but my try-params-nightly.yml optimization runs do.  (nothing vs `--include-nightly`)
Hm, the beta run on try doesn't have sm.  Let's turn optimization off for beta and release.
That seems to make the most sense.  At a guess, your try pushes in this case were including commits that caused SM to run.
Overall, this is the same patch with the addition of disabling optimization for beta and release.  I rewrote the commit message to explain what the patch does a bit better.
Attachment #8846925 - Attachment is obsolete: true
Attachment #8847255 - Flags: review?(dustin)
Comment on attachment 8847255 [details] [diff] [review]
nightly-tests3.diff

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

::: taskcluster/taskgraph/decision.py
@@ +38,5 @@
>          # Always perform optimization.  This makes it difficult to use try
>          # pushes to run a task that would otherwise be optimized, but is a
>          # compromise to avoid essentially disabling optimization in try.
>          'optimize_target_tasks': True,
> +        'include_nightly': True,

Sorry, I missed replying to this in the bug -- but this sounds like a convenience for local development that might be better included in your local parameters.yml -- or at least commented nicely to indicate that all appearances to the contrary, this does not include nightly jobs on try.
Attachment #8847255 - Flags: review?(dustin) → review+
(In reply to Dustin J. Mitchell [:dustin] from comment #71)
> ::: taskcluster/taskgraph/decision.py
> @@ +38,5 @@
> >          # Always perform optimization.  This makes it difficult to use try
> >          # pushes to run a task that would otherwise be optimized, but is a
> >          # compromise to avoid essentially disabling optimization in try.
> >          'optimize_target_tasks': True,
> > +        'include_nightly': True,
> 
> Sorry, I missed replying to this in the bug -- but this sounds like a
> convenience for local development that might be better included in your
> local parameters.yml -- or at least commented nicely to indicate that all
> appearances to the contrary, this does not include nightly jobs on try.

I think it's useful for anyone using other target_task_method decision tasks on try, a la https://gist.github.com/acmiyaguchi/f20c4f47e723b3a94ad9a3f82da457f9 ... this is how we tested the nightly graph before we had the date branch, and I imagine would be useful for others too.

Adding a comment sounds like a good idea; will do.
https://hg.mozilla.org/projects/jamun/rev/6e496bd8703196247d9e15ab519bc3e3863e8c79
bug 1344321 - add nightly test support. r=dustin a=release

https://hg.mozilla.org/projects/jamun/rev/d65271b06ae8323f6323382af8b33b9cecd48e1f
bug 1344321 - enable run-on-projects for nightlies. r=dustin a=release

https://hg.mozilla.org/projects/jamun/rev/b4c662ddd935c80094114e4173bad55787dabe90
bug 1344321 - disable windows, mac, {win32,win64,linux}-pgo builds+tests on beta. r=dustin a=release
https://hg.mozilla.org/integration/mozilla-inbound/rev/82593afdf85d100060154952a7e18702098becd6
bug 1344321 - enable run-on-projects for nightlies. r=dustin a=release

https://hg.mozilla.org/integration/mozilla-inbound/rev/3e129df4d4282def0b9e29f9258e0eee38083388
bug 1344321 - disable windows, mac, {win32,win64,linux}-pgo builds+tests on beta. r=dustin a=release
Pushed by asasaki@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/07be3f735793
fix flake8. r=bustage a=release DONTBUILD
status:

X uplift above 2 patches to m-c
X uplift above 2 patches to m-a
X enable sm jobs on m-b - figure out how; write patches
  X get review
  X land sm jobs patches on m-b (with flake8 fix)
  X land sm jobs patches on m-i (with flake8 fix)
  X land sm jobs patches on m-a (with flake8 fix)
X review for treeherder patch
  X merge treeherder patch
  X roll out treeherder patch? - looks like yes!
Status: NEW → RESOLVED
Closed: 3 years ago
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.