Open Bug 982691 Opened 6 years ago Updated 6 years ago

Manifest Destiny doesn't support auto-updating manifests

Categories

(Testing :: Mozbase, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

People

(Reporter: jgraham, Unassigned)

Details

For web-platform-tests I have a requirement to update the manifests when upstream tests are changed with the minimum possible manual work. With manifestdestiny this doesn't work for two reasons:

1) The implementation doesn't expose the raw AST of the manifest, only an evaluated form. Therefore you can't parse a manifest, edit it, and rewrite the result.

2) The way that conditionals are handled is incompatible with doing automatic updates. For example if I have

fail-if = os == 'linux' || os == 'osx'

and I get an update, after which the test starts passing on a particular subversion of osx e.g. 10.9, it's unclear how I have to adjust the expression to reflect the new results without blowing away the whole right hand side and starting again, which tends to remove any additions that a human has made to the condition.

For web-platform tests, an example of the current solution can be seen at [1]. The idea is that each key an have multiple conditionals linking it to a value. Then the update procedure is to match each conditional to a set of results. Conditionals that didn't match any results are left untouched. Conditionals that matched one or more results, all with the same status, are untouched and the status is set to the new status. Conditionals that matched multiple results which are not all the same are removed and replaced with a new auto-generated conditional. Results that don't match any existing conditional are given their own conditional. For example if I have

expected:
  if c1 == a and c2 == a: FAIL
  if c1 == b: PASS

and I get the results (c1,c2,status): (a,a,PASS), (a,b,FAIL), (b,a,ERROR), (b,b,PASS)

then I would get the first conditional updated to have PASS on the rhs, the second conditional would be removed and replaced with two new ones for b,a and b,b cases and an entirely new conditional would be added for the (a,b) case (in reality things are a little more complex than this because there can be an unconditional default, but the general idea is right).

This format makes is relatively easy to update the manifest in the face of differing results from multiple platforms, which is a requirement. However inventing an entirely new format just for web-platform-tests is obviously unfortunate.

[1] http://hg.mozilla.org/projects/cedar/file/9af41dfd75f0/testing/web-platform-tests/metadata/2dcontext/building-paths/canvas_complexshapes_beziercurveto_001.htm.ini
would something like this be easier:
[test1]
skip-if = os == 'win'
skip-if = os == 'osx'
skip-if = os == 'b2g'
  
so then when windows is fixed we can just remove the one line?  I am trying to simplify your proposal
Yes, being able to repeat the same LHS would work, although I need more than just skip-if; the manifest has to record the actual result of the test, so it would be more like

fail-if = os == 'win'
error-if = os == 'osx'
error-if = os == 'b2g'

But having the value on the lhs creates some problems (or at least weirdness); all these *-if conditions need to be evaluated at once so that something like

fail-if = true
error-if = os == 'win'

has a single expected result on all platforms.
(but, to be clear, I believe that once you make the adjustments I described your proposal is isomorphic to mine; the only difference is that I have key: conditional-value* and you have (value-key: condition)*)
(In reply to James Graham [:jgraham] from comment #0)
> 1) The implementation doesn't expose the raw AST of the manifest, only an
> evaluated form. Therefore you can't parse a manifest, edit it, and rewrite
> the result.

This bothers the hell out of me, and I'd love to see it fixed. It will require a bit of rework, but I don't think it should be all that hard. I had to add a crummy hack to get the values of DEFAULT out of the manifest, it would be nice to do that in a cleaner way.

> 2) The way that conditionals are handled is incompatible with doing
> automatic updates. For example if I have
> 
> fail-if = os == 'linux' || os == 'osx'
> 
> and I get an update, after which the test starts passing on a particular
> subversion of osx e.g. 10.9, it's unclear how I have to adjust the
> expression to reflect the new results without blowing away the whole right
> hand side and starting again, which tends to remove any additions that a
> human has made to the condition.

So you want to be able to pull in a new version of tests, run them, then compare the results against the expected results and automatically rewrite the expectations based on those results?
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #4)
> 
> So you want to be able to pull in a new version of tests, run them, then
> compare the results against the expected results and automatically rewrite
> the expectations based on those results?

Yes, this is what web-platform-tests does (see the implementation at [1]). The idea is that tests which have been added/changed since the last update will automatically have their new results accepted, but human intervention will be needed when tests get a different result but the test file itself hasn't changed.

[1] https://hg.mozilla.org/projects/cedar/file/b93697c8a763/testing/web-platform-tests/wpttests/manifestupdate.py
You need to log in before you can comment on or make changes to this bug.