It is occasionally useful to be able to ask "what TBPL chunk does this test run in?" With mochitest manifests and more and more TBPL logic moving into the tree, this question shouldn't be *too* difficult to answer... Bonus points for adding the capability to do this for some other platform than the one you're currently on and/or building Firefox for.

The only tricky bit here is that to know what chunk something runs in you have to know how many total chunks there are, and that information currently lives in buildbot or mozharness somewhere. If we could move that into the tree that would be fantastic. I know ahal ran into this when moving some mozharness configs into the tree. We could doubly-define this to get this working until we figure out how to make m-c the source of truth for it. I don't think we actually change chunking that often.

I think this would be really nice, but even nicer would be not having to care about chunks at all. If we had a more expressive try syntax that let you specify one or more test paths, we could have the automation just figure out what minimum set of chunks would be required to run all the specified tests. But this would definitely be a good first step. (In reply to Ted Mielczarek [:ted.mielczarek] from comment #1) > The only tricky bit here is that to know what chunk something runs in you > have to know how many total chunks there are, and that information currently > lives in buildbot or mozharness somewhere. If we could move that into the > tree that would be fantastic. I know ahal ran into this when moving some > mozharness configs into the tree. > > We could doubly-define this to get this working until we figure out how to > make m-c the source of truth for it. I don't think we actually change > chunking that often. I wonder if this would be easier to do with jobs running via taskcluster?

Hopefully this isn't sidetracking this bug, but a thing I've always wanted is for mochitest to easily run a given test. My current work around is to write a temporary manifest with only that bug.

s/bug/test/

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #3) > Hopefully this isn't sidetracking this bug, but a thing I've always wanted > is for mochitest to easily run a given test. My current work around is to > write a temporary manifest with only that bug. Do you mean in try? You should be able to add '--test-path=path/to/test' in the relevant in-tree mozharness config: http://mxr.mozilla.org/mozilla-central/source/testing/config/mozharness/ If you meant locally, just run 'mach mochitest-plain path/to/test'

Since last quarter some of the in-tree mozharness configs have the --total-chunks variable: https://dxr.mozilla.org/mozilla-central/search?q=%22--total-chunks%22&case=false

Bug 999450 - Add chunk_finder command in mach to discover which chunk a mochitest is present in for own platform.

Comment on attachment 8653871 [details] MozReview Request: Bug 999450 - Make find-test-chunk detect if a test is disabled on a platform. r=chmanchester This patch accurately determines the chunk for mochitest-plain and mochitest-browser-chrome for the platform same as the user's machine. To do this, I had to make 'mach mochitest' not filter tests before invoking harness, so that master manifest got passed down to manifestparser (like in production). Right now, the commands for this looks like: For mochitest-plain ./mach chunk_finder mochitest dom/inputmethod/mochitest/test_bug1137557.html -f plain --total-chunks 5 --chunk-by-dir 4 For mochitest-browser-chrome: ./mach chunk_finder mochitest browser/base/content/test/plugins/browser_plugins_added_dynamically.js -f browser-chrome --total-chunks 3 --chunk-by-runtime I will add more features (like make it work for other suites, eg. dt, e10s, able to determine chunks for different platforms, able to automatically detect --total-chunks) in other upcoming patches. Chris, can you give feedback on this patch?

https://reviewboard.mozilla.org/r/17549/#review15803 ::: testing/mochitest/mach_commands.py:466 (Diff revision 1) > - def run_mochitest_general(self, flavor=None, test_objects=None, **kwargs): > + def run_mochitest_general(self, flavor=None, test_objects=None, test_resolution=True, **kwargs): I would call this parameter "resolve_tests" ::: testing/mochitest/mach_commands.py:580 (Diff revision 1) > + # This is a hack to add an option in mach to prevent filtering > + # tests before sending to mochitest harness. > + if not test_resolution: > + tests = [] This doesn't quite seem like the way to do this... can we just skip the resolution/filtering steps and pass 'test_paths' in to harness_args instead of overriding the value? (The mochitest argument parser takes 'test_paths' these days). ::: tools/mach_commands.py:1 (Diff revision 1) > # This Source Code Form is subject to the terms of the Mozilla Public > # License, v. 2.0. If a copy of the MPL was not distributed with this > # file, # You can obtain one at http://mozilla.org/MPL/2.0/. Let's put this in testing/mach_commands.py or testing/tools/mach_commands.py ::: tools/mach_commands.py:24 (Diff revision 1) > +def parse_args(argv=None): Maybe call this get_parser? ::: tools/mach_commands.py:26 (Diff revision 1) > + parser.add_argument(dest="suite_name", > + nargs='+', > + type=str, > + help="The test for which chunk should be found.") > + > + parser.add_argument(dest="test_path", > + nargs='+', > + type=str, > + help="The test for which chunk should be found.") Do these need to be nargs='+'? I don't think you're using any but the first below. ::: tools/mach_commands.py:28 (Diff revision 1) > + type=str, > + help="The test for which chunk should be found.") Add a note in the documentation that this corresponds to the mach command invoked. ::: tools/mach_commands.py:96 (Diff revision 1) > + for test in tests: > + if test['path'] == test_path: > + print "The test %s is present in chunk number: %d." % (test_path, this_chunk) > + found = True > + break Instead of using the "found" flag variable, we could convert to a list of paths and test for inclusion: paths = [t['path'] for t in tests] if test_path in paths: ...

https://reviewboard.mozilla.org/r/17549/#review15815 ::: tools/mach_commands.py:26 (Diff revision 1) > + parser.add_argument(dest="suite_name", > + nargs='+', > + type=str, > + help="The test for which chunk should be found.") > + > + parser.add_argument(dest="test_path", > + nargs='+', > + type=str, > + help="The test for which chunk should be found.") I am using ./mach chunk_finder mochitest dom/inputmethod/mochitest/test_bug1137557.html -f plain --total-chunks 5 --chunk-by-dir 4 so suite_name == 'mochitest' and test_path == 'dom/inputmethod/mochitest/test_bug1137557.html' are being used with nargs.

A few drive-by thoughts...but feel free to ignore / don't let me slow you down. - Please use "chunk-finder" rather than "chunk_finder" (hyphen vs underscore) to be consistent with existing mach commands ("show-log", "check-spidermonkey"); - I prefer verb-centric command names: "find-chunks" instead of "chunk-finder"; - I prefer very-descriptive command names: "find-test-chunks" instead of "find-chunks"; - What happens if the specified test does not exist? I'd like an explicit "test not found" error. - What happens if the specified test is skipped on this platform? If it's marked as failing?

https://reviewboard.mozilla.org/r/17549/#review15815 > I am using > ./mach chunk_finder mochitest dom/inputmethod/mochitest/test_bug1137557.html -f plain --total-chunks 5 --chunk-by-dir 4 > > so suite_name == 'mochitest' and test_path == 'dom/inputmethod/mochitest/test_bug1137557.html' > are being used with nargs. I think they can both be nargs=1

Comment on attachment 8653871 [details] MozReview Request: Bug 999450 - Make find-test-chunk detect if a test is disabled on a platform. r=chmanchester Bug 999450 - Add find-test-chunk command in mach to discover the chunk for a mochitest on a platform.r=chmanchester

Comment on attachment 8653871 [details] MozReview Request: Bug 999450 - Make find-test-chunk detect if a test is disabled on a platform. r=chmanchester https://reviewboard.mozilla.org/r/17549/#review15905 ::: testing/mach_commands.py:585 (Diff revision 2) > + print("The test %s is present in chunk number: %d." % (test_path, this_chunk)) > + found = True :gbrown's comments reminded me this will include skipped tests, it would be good to include something in the message to that effect, and detect when a skipped test is specified as a follow up.

Comment on attachment 8653871 [details] MozReview Request: Bug 999450 - Make find-test-chunk detect if a test is disabled on a platform. r=chmanchester Bug 999450 - Add find-test-chunk command in mach to discover the chunk for a mochitest on a platform.r=chmanchester

Comment on attachment 8653871 [details] MozReview Request: Bug 999450 - Make find-test-chunk detect if a test is disabled on a platform. r=chmanchester https://reviewboard.mozilla.org/r/17549/#review15933 r+

https://reviewboard.mozilla.org/r/17549/#review15935 r+

Comment on attachment 8653871 [details] MozReview Request: Bug 999450 - Make find-test-chunk detect if a test is disabled on a platform. r=chmanchester Carrying forward the r+ from :chmanchester

Successful try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6e4884960e73

Comment on attachment 8653871 [details] MozReview Request: Bug 999450 - Make find-test-chunk detect if a test is disabled on a platform. r=chmanchester Bug 999450 - Make find-test-chunk detect if a test is disabled on a platform. r=chmanchester

Bug 999450 - Adding --e10s option to make find-test-chunk detect chunk for a test on e10s platform. r=chmanchester

Bug 999450 - Adding --subsuite option to make find-test-chunk detect chunk for tests run as subsuite. r=chmanchester

Bug 999450 - Adding --debug to make find-test-chunk detect chunk for a test for a debug build. r=chmanchester

Bug 999450 - Adding --platform to make find-test-chunk detect chunk for a test for another platform. r=chmanchester

Comment on attachment 8653871 [details] MozReview Request: Bug 999450 - Make find-test-chunk detect if a test is disabled on a platform. r=chmanchester https://reviewboard.mozilla.org/r/17549/#review16097 ::: testing/mach_commands.py:584 (Diff revision 4) > - if test_path in paths: > + if test_path in test['path']: Shouldn't this be "==", not "in"? ::: testing/mach_commands.py:585 (Diff revision 4) > - print("The test %s is present in chunk number: %d (it may be skipped)." % (test_path, this_chunk)) > + if 'disabled' in test: > + print("The test %s is disabled on the given platform." % test_path) > + else: > + print("The test %s is present in chunk number: %d." % (test_path, this_chunk)) We're still shy of evaluating "skip-if" conditions, which are used a lot more than 'disabled' in manifests, so I don't think this will cover what people usually think of as a disabled test.

Comment on attachment 8655548 [details] MozReview Request: Bug 999450 - Adding --e10s option to make find-test-chunk detect chunk for a test on e10s platform. r=chmanchester https://reviewboard.mozilla.org/r/17921/#review16099 ::: testing/mach_commands.py:551 (Diff revision 1) > + action="store_true", > + dest='e10s', A mix of single and double quotes crept in here somehow.

Comment on attachment 8655549 [details] MozReview Request: Bug 999450 - Adding --debug to make find-test-chunk detect chunk for a test for a debug build. r=chmanchester https://reviewboard.mozilla.org/r/17923/#review16101 ::: testing/mach_commands.py:559 (Diff revision 1) > + help='Subsuite of tests to run. Only one can be specified at once.', Update these help messages to say something about chunking tests for the given option instead of running them.

https://reviewboard.mozilla.org/r/17549/#review16097 > We're still shy of evaluating "skip-if" conditions, which are used a lot more than 'disabled' in manifests, so I don't think this will cover what people usually think of as a disabled test. I was mistaken about this -- 'disabled' gets set in filters.py.

Comment on attachment 8653871 [details] MozReview Request: Bug 999450 - Make find-test-chunk detect if a test is disabled on a platform. r=chmanchester https://reviewboard.mozilla.org/r/17549/#review16107

Comment on attachment 8655586 [details] MozReview Request: Bug 999450 - Adding --platform to make find-test-chunk detect chunk for a test for another platform. r=chmanchester https://reviewboard.mozilla.org/r/17943/#review16109 ::: testing/mach_commands.py:565 (Diff revision 1) > + parser.add_argument('--debug', > + action="store_true", Mix of ' and " ::: testing/mach_commands.py:569 (Diff revision 1) > + default=False) Be consistent about whether to include a trailing comma. ::: testing/mach_commands.py:614 (Diff revision 1) > + arguments = [ > + 'install', > + '-U', > + 'requests==2.7.0' > + ] This makes me a little nervous -- installing in the build virtualenv could cause version conflicts elsewhere. What else in the tree is using requests?

https://reviewboard.mozilla.org/r/17943/#review16113 ::: testing/mach_commands.py:614 (Diff revision 1) > + arguments = [ > + 'install', > + '-U', > + 'requests==2.7.0' > + ] I can see 'requests' used for install pip at only one place: https://dxr.mozilla.org/mozilla-central/search?q=install_pip_package&redirect=false&case=true&limit=63&offset=0

https://reviewboard.mozilla.org/r/17943/#review16109 > This makes me a little nervous -- installing in the build virtualenv could cause version conflicts elsewhere. What else in the tree is using requests? This upgrades existing requests from 2.5.3 to 2.7.0 requests.get is used in a number of places: https://dxr.mozilla.org/mozilla-central/search?q=requests.get&redirect=true&case=true&limit=63&offset=63 but looking at the release history: https://pypi.python.org/pypi/requests it looks like there have only been bugfixes, so (hopefully) nothing else should break.

I took a pip freeze on a fresh downloaded inbound repo before mozdownload was installed: futures (3.0.2) mock (1.0.0) pip (6.0.6) psutil (3.1.1) pyasn1 (0.1.7) pyasn1-modules (0.0.5) redo (1.4) requests (2.5.1) rsa (3.1.4) setuptools (11.0) None of the libraries has a version conflict for requests, and this time I was able to install mozdownload without any problem.

Comment on attachment 8653871 [details] MozReview Request: Bug 999450 - Make find-test-chunk detect if a test is disabled on a platform. r=chmanchester Bug 999450 - Make find-test-chunk detect if a test is disabled on a platform. r=chmanchester

Comment on attachment 8655548 [details] MozReview Request: Bug 999450 - Adding --e10s option to make find-test-chunk detect chunk for a test on e10s platform. r=chmanchester Bug 999450 - Adding --e10s option to make find-test-chunk detect chunk for a test on e10s platform. r=chmanchester

Comment on attachment 8655549 [details] MozReview Request: Bug 999450 - Adding --debug to make find-test-chunk detect chunk for a test for a debug build. r=chmanchester Bug 999450 - Adding --subsuite option to make find-test-chunk detect chunk for tests run as subsuite. r=chmanchester

Comment on attachment 8655586 [details] MozReview Request: Bug 999450 - Adding --platform to make find-test-chunk detect chunk for a test for another platform. r=chmanchester Bug 999450 - Adding --debug to make find-test-chunk detect chunk for a test for a debug build. r=chmanchester

Comment on attachment 8655620 [details] MozReview Request: Bug 999450 - Make find-test-chunk automatically detect test flavor and subsuite via TestResolver. r=chmanchester Bug 999450 - Adding --platform to make find-test-chunk detect chunk for a test for another platform. r=chmanchester

Comment on attachment 8653871 [details] MozReview Request: Bug 999450 - Make find-test-chunk detect if a test is disabled on a platform. r=chmanchester https://reviewboard.mozilla.org/r/17549/#review16145 r+

Comment on attachment 8655548 [details] MozReview Request: Bug 999450 - Adding --e10s option to make find-test-chunk detect chunk for a test on e10s platform. r=chmanchester Sorry for the spam. Mozreview applied review for chmanchester, even though he had already r+'ed it.

I lost track of what's landed and what hasn't, but I tested the command out yesterday and got an exception when no tests were found. Specifically without passing in -f, it defaulted to 'a11y' and error'ed out. If that's already been fixed, then please disregard. That being said, we have the ability to automatically determine which suite/flavor a test is in, would be nice if that wasn't required on the command line. If this feature got integrated into |mach test|, then I think a lot of this work would be done for us. Here's an example of what I'm thinking of: $ mach test --find path/to/test1 path/to/dir test | suite | chunk | --------------------------------------------- path/to/test1 | mochitest-plain | 2 | path/to/dir/test2 | xpcshell | 5 | path/to/dir/test3 | xpcshell | 5 | .... In the above --find is a 'store_true' action and the test paths are the same as normal. I'm of the opinion that we have too many mach commands for rare edge cases, so if it's possible to integrate this into an existing command like |mach test|, my preference would be to do that.

Though thinking about it, |mach test| would still need --total-chunks to be passed in :/.. would be nice if we just knew this information automatically, but that might not be possible until taskcluster.

I think having a |mach chunk-finder| command is probably ok for the short term. But the 95% use case that chunk-finder is trying to solve is developers trying to run the entire chunk that contains a given test on try (e.g to debug an intermittent, or to make sure a new test passes). While |mach chunk-finder| certainly makes that easier, it would be much more convenient if |mach try| could just do it automatically. I filed bug 1201072 for chunk-finder integration into |mach try|.

Successful try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f79792356f7e checkin-needed for the r+ patches, that is, https://bugzilla.mozilla.org/attachment.cgi?id=8653871 https://bugzilla.mozilla.org/attachment.cgi?id=8655548 https://bugzilla.mozilla.org/attachment.cgi?id=8655549

Bug 999450 - Make find-test-chunk search all mochitests in case flavor is not specified.

Comment on attachment 8656186 [details] MozReview Request: Bug 999450 - Make find-test-chunk automatically detect test flavor via TestResolver. r=chmanchester Andrew, I agree that if the flavor of test is not given then we should search in all the flavors. We can do that in the current scenario by iterating over a list of all given flavors. Attached a patch for this. I think it is still early for find-test-chunk to be integrated in 'mach test', since it works only for mochitest and we also need the --total-chunks to be passed by the user.

https://reviewboard.mozilla.org/r/17943/#review16109 > This upgrades existing requests from 2.5.3 to 2.7.0 > requests.get is used in a number of places: https://dxr.mozilla.org/mozilla-central/search?q=requests.get&redirect=true&case=true&limit=63&offset=63 > but looking at the release history: https://pypi.python.org/pypi/requests it looks like there have only been bugfixes, so (hopefully) nothing else should break. Ok, 2.5.3 comes from a copy of requests in the tree under python/requests. I think the absolute best way to deal with this would be to update the in-tree requests, but it looks like it was imported for something that runs in automation so the current approach will not cause problems.

Comment on attachment 8655586 [details] MozReview Request: Bug 999450 - Adding --platform to make find-test-chunk detect chunk for a test for another platform. r=chmanchester https://reviewboard.mozilla.org/r/17943/#review16215

Comment on attachment 8655620 [details] MozReview Request: Bug 999450 - Make find-test-chunk automatically detect test flavor and subsuite via TestResolver. r=chmanchester https://reviewboard.mozilla.org/r/17949/#review16217 ::: testing/mach_commands.py:561 (Diff revision 2) > + choices=['linux', 'linux64', 'mac', 'mac64', 'win32', 'win64'], Can we make this 'macosx64' so it's consistent with try syntax? ::: testing/mach_commands.py:584 (Diff revision 2) > + if platform: > + args.extend(['-p', platform]) Just curious, how does this work in the case we have debug but no platform is provided? Does mozdownload provide a default or do something intelligent based on the platform its run on?

https://reviewboard.mozilla.org/r/17949/#review16217 > Just curious, how does this work in the case we have debug but no platform is provided? Does mozdownload provide a default or do something intelligent based on the platform its run on? if we have --debug and no platform is provided, mozdownload will download mozinfo for debug type of the user's host platform.

https://reviewboard.mozilla.org/r/17949/#review16217 > Can we make this 'macosx64' so it's consistent with try syntax? I am keeping it same as used in mozdownload: https://github.com/mozilla/mozdownload/blob/master/mozdownload/scraper.py#L46 Otherwise I would have to convert it to 'mac64' before passing to mozdownload

https://reviewboard.mozilla.org/r/18103/#review16223 ::: testing/mach_commands.py:598 (Diff revision 1) > - flavor = kwargs['flavor'] > + if kwargs['flavor']: > + flavors = [kwargs['flavor']] > + else: > + flavors = ['a11y', 'jetpack-addon', 'mochitest', 'browser-chrome',\ > + 'jetpack-package', 'webapprt-chrome', 'chrome', 'webapprt-content'] I think it would be preferable to get rid of the flavor option altogether and determine the flavor based on all-tests.json. This can be done with the test resolver -- if you just call it with a path to a test it will return an object with a 'flavor' key. There are comments in python/mozbuild/mozbuild/testing.py that describe the api to use.

Comment on attachment 8655620 [details] MozReview Request: Bug 999450 - Make find-test-chunk automatically detect test flavor and subsuite via TestResolver. r=chmanchester Bug 999450 - Adding --platform to make find-test-chunk detect chunk for a test for another platform. r=chmanchester

Comment on attachment 8656186 [details] MozReview Request: Bug 999450 - Make find-test-chunk automatically detect test flavor via TestResolver. r=chmanchester Bug 999450 - Make find-test-chunk automatically detect test flavor via TestResolver. r=chmanchester

Comment on attachment 8655620 [details] MozReview Request: Bug 999450 - Make find-test-chunk automatically detect test flavor and subsuite via TestResolver. r=chmanchester Carrying forward the r+

Successful try run for latest patches: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e4ec27c16c88 Checkin-needed for r+ patches.

Comment on attachment 8656186 [details] MozReview Request: Bug 999450 - Make find-test-chunk automatically detect test flavor via TestResolver. r=chmanchester https://reviewboard.mozilla.org/r/18103/#review16363 ::: testing/mach_commands.py:605 (Diff revision 2) > + raise Exception('The number of tests for the given test path is not equal to 1.' > + 'Please enter the test path correctly.') > + Let's make the message a little simpler -- just say no tests were found. Also not sure an exception is the best user experience here, you can just print the error and exit(1) ::: testing/mach_commands.py:615 (Diff revision 2) > 'subsuite': kwargs['subsuite'], As we discussed we can probably get the subsuite out of the test resolver now and remove that option.

Comment on attachment 8655549 [details] MozReview Request: Bug 999450 - Adding --debug to make find-test-chunk detect chunk for a test for a debug build. r=chmanchester Bug 999450 - Adding --debug to make find-test-chunk detect chunk for a test for a debug build. r=chmanchester

Comment on attachment 8655586 [details] MozReview Request: Bug 999450 - Adding --platform to make find-test-chunk detect chunk for a test for another platform. r=chmanchester Bug 999450 - Adding --platform to make find-test-chunk detect chunk for a test for another platform. r=chmanchester

Comment on attachment 8655620 [details] MozReview Request: Bug 999450 - Make find-test-chunk automatically detect test flavor and subsuite via TestResolver. r=chmanchester Bug 999450 - Make find-test-chunk automatically detect test flavor and subsuite via TestResolver. r=chmanchester

Comment on attachment 8655549 [details] MozReview Request: Bug 999450 - Adding --debug to make find-test-chunk detect chunk for a test for a debug build. r=chmanchester Carrying forward r+

Comment on attachment 8655586 [details] MozReview Request: Bug 999450 - Adding --platform to make find-test-chunk detect chunk for a test for another platform. r=chmanchester Carrying forward r+

Comment on attachment 8655620 [details] MozReview Request: Bug 999450 - Make find-test-chunk automatically detect test flavor and subsuite via TestResolver. r=chmanchester Carrying forward r+

CC'ing Wes and Tomcat, the patches are r+ (carry forward), and there are successful try runs pasted before (although these patches don't really require a try run). Checkin-needed for all of them.

(In reply to Vaibhav (:vaibhav1994) from comment #66) > CC'ing Wes and Tomcat, the patches are r+ (carry forward), and there are > successful try runs pasted before (although these patches don't really > require a try run). Checkin-needed for all of them. Hi, did the checkin, sorry for the delay! Cheers!

https://hg.mozilla.org/integration/mozilla-inbound/rev/343266cde83b https://hg.mozilla.org/integration/mozilla-inbound/rev/31157eaf6f0a https://hg.mozilla.org/integration/mozilla-inbound/rev/04c8bc7da16d https://hg.mozilla.org/integration/mozilla-inbound/rev/f3b53e94793c https://hg.mozilla.org/integration/mozilla-inbound/rev/701f6198a5ba

https://hg.mozilla.org/mozilla-central/rev/343266cde83b https://hg.mozilla.org/mozilla-central/rev/31157eaf6f0a https://hg.mozilla.org/mozilla-central/rev/04c8bc7da16d https://hg.mozilla.org/mozilla-central/rev/f3b53e94793c https://hg.mozilla.org/mozilla-central/rev/701f6198a5ba

Some features and the basic structure for find-test-chunk was added in this bug. Need to file more bugs for others features. Resolving this.

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