369330 - allow reftests to be marked as failing or random on a specific platform

Status: RESOLVED FIXED

(Testing :: Reftest, defect)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

After having gone through the platform differences in reftest success, I'd like to add to reftest the ability to mark a test as currently failing or currently random based on the platform.

In particular, what I propose is getting rid of the "f" at the beginning of the test operator and replacing it by putting failure or randomness status *before* the operator.  (It's prominent there in the manifest, and easier to process if we extend the test types.)  So I'd propose a syntax something like:

== testurl ref-url # just like now
fails == testurl ref-url # test that fails everywhere
fails-if(platform==Windows) == testurl ref-url
random-if(platform==Windows) == testurl ref-url # for things like bug 369040

Anything marked as random will have its PASS/FAIL status in the output along with "(MARKED AS RANDOM)".

I'm not yet sure what I want for the syntax inside the ().  I might find some way to test based on stuff in autoconf.mk and/or mozilla-config.h, although I might just hack in navigator.platform tests for now.

Thoughts?

Comment 1

[Jeff Walden [:Waldo]]

Bug 369017 would handle everything except randomness, and it'd also enable other build-type tests beyond just platform.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

But that way, it would be a pain to write

#if foo
== ... ...
#else
f== ... ...
#endif

which is generally what we want, since we want to know if a failing test becomes a passing test so that we are forced to change the manifest and then avoid regressing it.

Comment 3

[Rob Campbell [:rc] (:robcee)]

I think I prefer the manifest language in dbaron's original comment to Waldo's example. This is a necessary addition, imo, as the tests on windows are currently useless.

Alternatively, another approach would be to include multiple manifests for each platform. Linux, Mac and Windows. This would pass the pain onto the person who has to update 3 manifest files to add a test, but marking one failing on one or more platform would be fairly easy. It has the added benefit of not adding to the manifest "language".

Take your pick, David.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

This works, but I'd like to tweak the code a bit more, and I haven't added fails-if / random-if lines for Mac and Windows yet.

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

OK, I got it working with evalInSandbox, which solved the issue I was worrying about with the possibility of an autoconf.mk variable that conflicted with my variable -- in theory it's also more secure if we ever had untrusted reftest manifests, although I don't think we should.  I haven't actually tested the security aspects of it, and I'm not worried about that possibility.

In any case, this removes the "f"s from the beginning of the test operands and adds fails, random, fails-if(), and random-if() keywords that go at the beginning of a test line.  The contents of fails-if and random-if are javascript expressions, to be evaluated when all of the "simple" variables in autoconf.mk are defined (all as strings).

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Note that this also doesn't make the necessary annotations for Windows and Mac; I'll do that after this lands.  One step at a time...

Comment 7

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 254217 [details] [diff] [review]
patch

I might change (RESULT EXPECTED TO BE RANDOM) to something more succinct. Like RANDOM or DON'T CARE, but, r+.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Well, I want the randomness to be pretty obvious for two reasons:

 1. having to mark something as random is something that we should try to avoid; fixing nondeterministic behavior should be a priority.

 2. when something is marked as random, it will never output "UNEXPECTED" and thus turn tinderbox orange.  So we should make it more noticeable in other ways.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Checked in to trunk, 2007-02-08 11:24 -0800 with various manifest followups over the course of the day.

I think things should be passing now on Linux, Mac, and Windows.  The linux testing tinderbox actually was green, although the Windows one (and the Linux one) went offline right before it would have picked up the last manifest change needed for Windows.

Comment 10

[Rob Campbell [:rc] (:robcee)]

It looks good, David. We have a nice green pair of boxes now. Mac to follow soon...