Closed Bug 1403322 Opened 7 years ago Closed 6 years ago

Switch jsreftests, jittests, test-verify to SCHEDULES

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla58

People

(Reporter: dustin, Assigned: dustin)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 4 obsolete files)

These currently have a custom "when" clause
Summary: Switch jsreftests, jittests to SCHEDULES → Switch jsreftests, jittests, test-verify to SCHEDULES
This will need to land atop bug 1403222 at least to avoid conflicts.

I think the approach here is to remove these suites from the "exclusive" list and add them to the "inclusive" suite.  This will match the current behavior, and not run the jittests unless either js/{src,public}/** or any of the jittest test harness changes.  If there were test harnesses defined (I don't see any in bug 1403222) those could still be exclusive.
Depends on: 1403222
Comment on attachment 8917469 [details]
Bug 1403322 - schedule jittests inclusively;

https://reviewboard.mozilla.org/r/188436/#review194008
Attachment #8917469 - Flags: review?(ahalberstadt) → review+
Comment on attachment 8917470 [details]
Bug 1403322 - schedule jsreftests inclusively;

https://reviewboard.mozilla.org/r/188438/#review194010
Attachment #8917470 - Flags: review?(ahalberstadt) → review+
Comment on attachment 8917471 [details]
Bug 1403322 - schedule test-verification inclusively;

https://reviewboard.mozilla.org/r/188440/#review194012
Attachment #8917471 - Flags: review?(ahalberstadt) → review+
Comment on attachment 8917472 [details]
Bug 1403322 - remove support for when.files-changed in tests;

https://reviewboard.mozilla.org/r/188442/#review194014

::: taskcluster/taskgraph/transforms/tests.py:367
(Diff revision 1)
>      'test-name': basestring,
>  
>      # the product name, defaults to firefox
>      Optional('product'): basestring,
>  
> -    # conditional files to determine when these tests should be run
> +    Optional('when'): {

More of a thought than a request.. Should we just get rid of `when` entirely? Calling the key `skip-unless-schedules` will make it more obvious how this is all wired together.

Feel free to ignore or handle in a follow-up.
Attachment #8917472 - Flags: review?(ahalberstadt) → review+
(In reply to Andrew Halberstadt [:ahal] from comment #10)
> More of a thought than a request.. Should we just get rid of `when`
> entirely? Calling the key `skip-unless-schedules` will make it more obvious
> how this is all wired together.

So instead of

  when:
    schedules: [..]

tests would contain

  optimization:
    skip-unless-schedules: [..]

Yeah, I thought about that, and since you mentioned it too I suppose it's a good idea.  I'll redraft.
Hm, actually, it's sometimes skip-unless-schedules-or-seta (for non-try branches).  So transparently using `optimization` is kind of messy.
I think I'll leave this as-is, actually.
Pushed by dmitchell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5c50f100460d
schedule jittests inclusively; r=ahal
https://hg.mozilla.org/integration/autoland/rev/547109f6eb1d
schedule jsreftests inclusively; r=ahal
https://hg.mozilla.org/integration/autoland/rev/e3a310b6b896
schedule test-verification inclusively; r=ahal
https://hg.mozilla.org/integration/autoland/rev/ccc4f12edef0
remove support for when.files-changed in tests; r=ahal
Blocks: fastci
:dustin - Recently I see test-verify running even when js/html/xul/xhtml files are not changed. For example:

https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=123965af3a606397e7b0f5705ae3365ad64a1258&filter-searchStr=test-verify

Did something go wrong here? I notice your change references 'test-verification'; should that be 'test-verify'?
Flags: needinfo?(dustin)
Depends on: 1410911
Filed bug 1410911 for this issue.
Flags: needinfo?(dustin)
This was backed out in bug 1410911, and will need to be re-landed, after bug 1426254 is solved.
Status: RESOLVED → REOPENED
Depends on: 1426254
Resolution: FIXED → ---
OK, I have an updated patch for this that looks good in my testing (produces tasks with the right optimization set) based on bug 1426254.  Once that patch lands, I'll put this one up for review.  Then on to bug 1403519.
Attachment #8917469 - Attachment is obsolete: true
Attachment #8917470 - Attachment is obsolete: true
Attachment #8917471 - Attachment is obsolete: true
Attachment #8917472 - Attachment is obsolete: true
Comment on attachment 8940785 [details]
Bug 1403322: Switch jsreftests, jittests, test-verify to SCHEDULES;

https://reviewboard.mozilla.org/r/211058/#review216850

Thanks, lgtm!

::: python/mozbuild/mozbuild/schedules.py:18
(Diff revision 1)
>  
>  INCLUSIVE_COMPONENTS = [
>      'py-lint',
>      'js-lint',
>      'yaml-lint',
> +    # inclusive test suites -- thes *only* run when certain files have changed

nit: these
Attachment #8940785 - Flags: review?(ahalberstadt) → review+
Pushed by dmitchell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/976c7e62b637
Switch jsreftests, jittests, test-verify to SCHEDULES; r=ahal
https://hg.mozilla.org/mozilla-central/rev/976c7e62b637
Status: REOPENED → RESOLVED
Closed: 7 years ago6 years ago
Resolution: --- → FIXED
This removes support for `when.files-changed` from tests, which thunderbird is still using (until bug 1418882 gets fixed).
Depends on: 1429236
Comment on attachment 8941163 [details]
Backout removal of when.files-changed support for tests from Bug 1403322;

https://reviewboard.mozilla.org/r/211430/#review217484

Let's leave this bug open and re-land this bit once TB is OK with it..
Attachment #8941163 - Flags: review?(dustin) → review+
Keywords: leave-open
Backout by mozilla@hocat.ca:
https://hg.mozilla.org/integration/autoland/rev/a6fb6d5f3b5e
Backout removal of when.files-changed support for tests from Bug 1403322; r=dustin
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.