Closed Bug 687264 Opened 10 years ago Closed 10 years ago

add mochitest support for filtering tests


(Testing Graveyard :: GoFaster, defect)

Not set


(Not tracked)



(Reporter: joel.maher, Assigned: jmaher)



(1 file, 1 obsolete file)

one idea that we can easily do is to only run tests which have been known to fail instead of all tests which include thousands which have never failed.

This is a simple solution where we take a commandline argument in python to a json formatted file and this is read inside the mochitest harness.
Attachment #560719 - Flags: feedback?(jgriffin)
Comment on attachment 560719 [details]
support mochitest filtering at runtime (WIP)

This is cool.   It's almost like someone is plotting to add manifest support to mochitest or something...
Attachment #560719 - Flags: feedback?(jgriffin) → feedback+
Ever confirmed: true
this patch supports directories and is tested a bit more.

Here is an example json file:
{"tests/dom/tests/mochitest/dom-level1-core": "",
 "tests/dom/tests/mochitest/dom-level2-core": "",
 "tests/dom/tests/mochitest/dom-level2-html": ""}

The idea is that we can put meta data in the value side of each key.  I don't want to argue on formats, assume json and give contructive feedback on the structure of the json if there is some to give:)

Currently this works with mochitest-plain, so we should decide if we want to expand into chrome/browser-chrome harness land.
Assignee: nobody → jmaher
Attachment #560719 - Attachment is obsolete: true
Attachment #561572 - Flags: review?(ctalbert)
Comment on attachment 561572 [details] [diff] [review]
support mochitest filtering at runtime (1.0)

Review of attachment 561572 [details] [diff] [review]:

Let me be sure I understand you - So the JSON right now is just a list of files.  Perhaps in the future we'll put metadata in that right hand value side of the JSON string?  That seems fine.

Is that tests/ directory you have prepended to the example necessary?  You're making a list of tests which are going to be interpreted as URLs by mochitest, so I think that it shouldn't be necessary.  Personally, I prefer having "tests/" prepended because that is how things are laid out on disk, but I worry we're introducing another concept of "path" given that if you specify a test directly, you leave off the "tests/" string (i.e. --test-path).  I think in this case, we should follow the paradigm that the --test-path abomination has already laid down and drop the tests/ string (if it is even required, doesn't look like it is).   

Other than that, I think the code looks good.  I worry a little about the xmlhttprequest call to get the filter file - if that were to fail it would hang mochitest since it's synchronous, and I'm not sure we'd have a good error message there.  But since we check for the existence of the file in python and since we would already have thrown if we didn't have a working webserver, it feels like quite a corner case and I think it's ok to let it go unless you have a brilliant idea to catch that error.

r+ with the nit about 'tests/'
Attachment #561572 - Flags: review?(ctalbert) → review+
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla9
Comment on attachment 561572 [details] [diff] [review]
support mochitest filtering at runtime (1.0)

>--- a/testing/mochitest/
>+++ b/testing/mochitest/
>     self.add_option("--repeat",
>                     action = "store", type = "int",
>                     dest = "repeat", metavar = "REPEAT",
>-                    help = "repeats the test or set of tests the given number of times, ie: repeat=1 will run the test twice.")
>+                    help = "repeats the test or set of tests the given number of times, ie: repeat=1 will run the test twice.")                   

Of course you were already planning to remove the trailing whitespace you added here.
Product: Testing → Testing Graveyard
You need to log in before you can comment on or make changes to this bug.