The documentation pages here: https://ci.mozilla.org/job/mozilla-central-docs/Build_Documentation/test_manifests.html#manifest-destiny-manifests and here: https://ci.mozilla.org/job/mozilla-central-docs/Build_Documentation/mozinfo.html#mozinfo-attributes and here: http://mozbase.readthedocs.org/en/latest/manifestdestiny.html and even the comment here: http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/manifestdestiny/manifestparser/manifestparser.py#680 Seem to indicate that this: [foo.js] skip-if = os == mac Should work in a browser.ini test manifest. It doesn't. You need quotes around the OS, which is also, ironically, what the example in http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/manifestdestiny/manifestparser/manifestparser.py#680 uses. Please either fix all this documentation (and any existing consumers with broken syntax), or fix the parser so that it works as advertised.
The docs are wrong :-(
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #1) > The docs are wrong :-( Alright, changed the summary.
Summary: skip-if = os == mac doesn't work, skip-if = os == "mac" does work → Build documentation about skip-if = os == whatever is wrong (should include quotes around 'whatever')
Sphinx supports doctest integration (http://sphinx-doc.org/ext/doctest.html). So, we could have Sphinx test our code examples as it builds the docs. I can say from experience that this is an insanely useful practice. These higher-level tests tend to complement lower-level unit tests nicely. If we did this, there is a stronger case for docs being part of the build. We don't want bad docs to propagate! This would mean importing Sphinx and some other dependencies in the tree. Annoying, but not a deal breaker. This could be a followup, however.
FWIW, I am not opposed to making the magic variables, um, magic variables (e.g. 'mac = "mac"'). I also have no problem not doing this ;) And yes, the docs should be fixed.
This almost tripped me up today - are we updating the docs to use quotes or supporting both?
Assignee: nobody → emorley
Status: NEW → ASSIGNED
Comment on attachment 823354 [details] [diff] [review] Fix docs Review of attachment 823354 [details] [diff] [review]: ----------------------------------------------------------------- ::: build/docs/test_manifests.rst @@ +62,5 @@ > There is a special **DEFAULT** section whose keys/metadata apply to all > sections/tests:: > > [DEFAULT] > + property = "value" This doesn't need quotes, this is not an expression. ::: testing/mozbase/manifestdestiny/manifestparser/manifestparser.py @@ -675,5 @@ > > def filter(self, values, tests): > """ > filter on a specific list tag, e.g.: > - run-if = os == win linux Ugh, this must have been a historical accident.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #7) > > [DEFAULT] > > + property = "value" > > This doesn't need quotes, this is not an expression. Oh ha yeah oops. Fixed that and landed: remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/3a33c6c8dfd0
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
You need to log in before you can comment on or make changes to this bug.