Nathan Froyd [:froydnj] Reporter
	Description • 10 years ago

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.

	(not currently active) Ted Mielczarek
	Comment 1 • 10 years ago

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.

	Andrew Halberstadt [:ahal]
	Comment 2 • 10 years ago

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?

	Brad Lassey [:blassey] (use needinfo?)
	Comment 3 • 10 years ago

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.

	Brad Lassey [:blassey] (use needinfo?)
	Comment 4 • 10 years ago

s/bug/test/

	Andrew Halberstadt [:ahal]
	Comment 5 • 10 years ago

(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'

	Armen [:armenzg]
	Comment 6 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 7 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 8 • 9 years ago

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?

	Chris Manchester (limited bugmail, email directly)
	Comment 9 • 9 years ago

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:
        ...

	Vaibhav (:vaibhav1994) Assignee
	Comment 10 • 9 years ago

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.

	Geoff Brown [:gbrown]
	Comment 11 • 9 years ago

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?

	Chris Manchester (limited bugmail, email directly)
	Comment 12 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 13 • 9 years ago

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

	Chris Manchester (limited bugmail, email directly)
	Comment 14 • 9 years ago

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.

	Vaibhav (:vaibhav1994) Assignee
	Comment 15 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 16 • 9 years ago

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+

	Vaibhav (:vaibhav1994) Assignee
	Comment 17 • 9 years ago

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

r+

	Vaibhav (:vaibhav1994) Assignee
	Comment 18 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 19 • 9 years ago

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

	Pulsebot
	Comment 20 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/4843e223c10e

	Carsten Book [:Tomcat]
	Comment 21 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/4843e223c10e

	Vaibhav (:vaibhav1994) Assignee
	Comment 22 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 23 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 24 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 25 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 26 • 9 years ago

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

	Chris Manchester (limited bugmail, email directly)
	Comment 27 • 9 years ago

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.

	Chris Manchester (limited bugmail, email directly)
	Comment 28 • 9 years ago

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.

	Chris Manchester (limited bugmail, email directly)
	Comment 29 • 9 years ago

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.

	Chris Manchester (limited bugmail, email directly)
	Comment 30 • 9 years ago

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.

	Chris Manchester (limited bugmail, email directly)
	Comment 31 • 9 years ago

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

	Chris Manchester (limited bugmail, email directly)
	Comment 32 • 9 years ago

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?

	Vaibhav (:vaibhav1994) Assignee
	Comment 33 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 34 • 9 years ago

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.

	Vaibhav (:vaibhav1994) Assignee
	Comment 35 • 9 years ago

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.

	Vaibhav (:vaibhav1994) Assignee
	Comment 36 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 37 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 38 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 39 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 40 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 41 • 9 years ago

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+

	Vaibhav (:vaibhav1994) Assignee
	Comment 42 • 9 years ago

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.

	Andrew Halberstadt [:ahal]
	Comment 43 • 9 years ago

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.

	Andrew Halberstadt [:ahal]
	Comment 44 • 9 years ago

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.

	Andrew Halberstadt [:ahal]
	Comment 45 • 9 years ago

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|.

	Vaibhav (:vaibhav1994) Assignee
	Comment 46 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 47 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 48 • 9 years ago

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.

	Chris Manchester (limited bugmail, email directly)
	Comment 49 • 9 years ago

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.

	Chris Manchester (limited bugmail, email directly)
	Comment 50 • 9 years ago

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

	Chris Manchester (limited bugmail, email directly)
	Comment 51 • 9 years ago

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?

	Vaibhav (:vaibhav1994) Assignee
	Comment 52 • 9 years ago

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.

	Vaibhav (:vaibhav1994) Assignee
	Comment 53 • 9 years ago

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

	Chris Manchester (limited bugmail, email directly)
	Comment 54 • 9 years ago

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.

	Vaibhav (:vaibhav1994) Assignee
	Comment 55 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 56 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 57 • 9 years ago

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+

	Vaibhav (:vaibhav1994) Assignee
	Comment 58 • 9 years ago

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

	Chris Manchester (limited bugmail, email directly)
	Comment 59 • 9 years ago

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.

	Vaibhav (:vaibhav1994) Assignee
	Comment 60 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 61 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 62 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 63 • 9 years ago

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+

	Vaibhav (:vaibhav1994) Assignee
	Comment 64 • 9 years ago

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+

	Vaibhav (:vaibhav1994) Assignee
	Comment 65 • 9 years ago

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+

	Vaibhav (:vaibhav1994) Assignee
	Comment 66 • 9 years ago

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.

	Carsten Book [:Tomcat]
	Comment 67 • 9 years ago

(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!

	Pulsebot
	Comment 68 • 9 years ago

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

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 69 • 9 years ago

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

	Vaibhav (:vaibhav1994) Assignee
	Comment 70 • 9 years ago

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.

	Firefox Bug Husbandry Bot
	Comment 71 • 6 years ago

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