Since approximately bug 1403322, test-verify (TV) runs on every push (unless the build has been coalesced or otherwise excluded), rather than only when a .html/.js/.xul/.xhtml file is modified.

A quick but speculative fix. This does no harm: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0d9634fac5f86fdf981b84a3340133b8d0b9ba2e and I prefer having just the one "test-verify" name in play, but I don't know if it eliminates the extra runs.

Comment on attachment 8921116 [details] [diff] [review] use 'test-verify' consistently Review of attachment 8921116 [details] [diff] [review]: ----------------------------------------------------------------- Sorry, I think that was my fault.

Pushed by gbrown@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/37db725b2308 Only run test-verify when certain file types are changed; r=dustin

It looks like my change is not fixing this problem. (But I don't mind keeping it / no need to backout.) Looking more closely, other test suites, like jit and jsreftest, are also running on most pushes, seemingly regardless of which files have changed. I think it is a regression from bug 1403322, but I don't see what's going wrong. Consider: https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-searchStr=jsref&tochange=1da1df814ad3bcb7aefb5f2c00c18b3c55f72284&fromchange=4dc78384cb58c67de770ef207bbd700e7a982960 :dustin -- Can you take this?

I'm at a meeting all week, so not quickly. It might be a start to look at the SCHEDULES components that are determined in the decision task -- do those include test-verify more often than they should?

(In reply to Dustin J. Mitchell [:dustin] from comment #5) > It might be a start to look at the SCHEDULES components that are determined > in the decision task -- do those include test-verify more often than they > should? I'm not sure I know how to answer that. I had a look at SkipUnlessSchedules' should_remove_task(). For a push with only a couple of python files modified, and considering a task like "test-linux64/opt-test-verify-e10s", should_remove_task() sees: scheduled == set([u'robocop', u'geckoview', u'reftest', u'web-platform-tests', u'macosx', u'talos', u'cppunittest', u'awsy', u'mochitest', u'web-platform-tests-reftests', u'web-platform-tests-wdspec', u'linux', u'telemetry-tests-client', u'windows', u'marionette', u'firefox-ui', u'gtest', u'xpcshell', u'android', u'xpcshell-coverage']) conditions == set(['test-verify', 'linux']) so scheduled & conditions == set(['linux']) and should_remove_task() returns false.

I have a feeling that we can't combine SCHEDULES.inclusive with SCHEDULES.exclusive. Consider test-verify. It has a it's own SCHEDULES.inclusive tag which should govern when it gets optimized or not. However, it also has an OS family (one of linux, macosx, windows or android). This means that this statement will always return True and the task will never be optimized: https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/optimize.py#386 Maybe there's something clever we can do here to better distinguish between inclusive and exclusive components, but maybe the cleanest option is to have two separate optimization: "skip-unless-schedules-exclusive" and "skip-unless-schedules-inclusive", and each task would need to decide which one to use.

I had thought that we might be able to just change: scheduled = self.scheduled_by_push(params['head_repository'], params['head_rev']) conditions = set(conditions) - # if *any* of the condition components are scheduled, do not optimize - if conditions & scheduled: + # if *all* of the condition components are scheduled, do not optimize + if conditions & scheduled == conditions: return False Indeed, that seems to fix the jittest and jsreftest cases, and possibly some test-verify issues. But, especially with test-verify, there can be a conflict between inclusive and exclusive. For instance, testing/mochitest has SCHEDULES.exclusive = ['mochitest', 'robocop'] But that directory also contains .js/.html test files that should trigger test-verify: with Files("**/*.js"): SCHEDULES.inclusive += ['test-verify'] with Files("**/*.html"): SCHEDULES.inclusive += ['test-verify'] and that gets confusing: mach file-info schedules testing/mochitest/tests/Harness_sanity/test_spawn_task.html robocop, mochitest

Yeah, `conditions & schedules == conditions` won't work. I stared at this for quite some time and I'm pretty sure the only way to properly solve it is to split inclusive and exclusive into two separate optimizations.

Your reasoning makes sense, and I feel silly for not seeing it.

:dustin - It sounds like you are the best person for this...hope you have the time.

We continue to run lots of unnecessary tests -- Android jsreftest alone is an extra 140 tasks per push on central (mitigated by SETA elsewhere). Can we back out bug 1403322, or fall back to when: files-changed?

If you think that's not too risky right now, please feel free.

Comment on attachment 8927400 [details] Bug 1410911 - back out bug 1403322; https://reviewboard.mozilla.org/r/198704/#review203820 Thanks. This looks right. I think the reward justifies the risk here, but you have a more complete perspective: I understand if you want to wait before checking in.

Pushed by dmitchell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c1848b8de242 back out bug 1403322; r=gbrown

OK, the complaint of this bug was fixed by the backout. I'll re-land in bug 1403322, with the fix in bug 1426254.

Removing leave-open keyword from resolved bugs, per :sylvestre.