Stand up reftest selftests

RESOLVED FIXED in Firefox 57

Status

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ahal, Assigned: ahal)

Tracking

(Blocks 1 bug)

unspecified
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

Attachments

(2 attachments)

We can leverage the work done for mochitest selftests and do something similar for reftest. To start we should check that failures/crashes/assertions/leaks turn the job orange.

See bug 1048446 for how to approach this.
Blocks: 1392391
Blocks: 1353461
Blocks: 1396901
Blocks: 1399155
Comment on attachment 8907086 [details]
Bug 1392390 - Refactor common code out of mochitest selftests and into a new moztest.selftest module,

https://reviewboard.mozilla.org/r/178798/#review183966

a bit of rubber stamping here- looks like a good refactor.
Attachment #8907086 - Flags: review?(jmaher) → review+
Comment on attachment 8907087 [details]
Bug 1392390 - Create a reftest selftest harness,

https://reviewboard.mozilla.org/r/178800/#review183968

please file a followup for the additional tests, the other nits might be easy to explain.

::: layout/tools/reftest/selftest/files/reftest-pass.list:3
(Diff revision 1)
> +== green.html green.html
> +== red.html red.html
> +!= green.html red.html

it owuld be nice to test more features in the .list files, a few come to mind:
http
skip-if
fuzzy-if
random-if
# commented out tests
include submanifests

::: taskcluster/ci/source-test/python.yml:168
(Diff revision 1)
> +        by-platform:
> +            linux64.*: aws-provisioner-v1/gecko-t-linux-xlarge
> +    worker:
> +        by-platform:
> +            linux64.*:
> +                docker-image: {in-tree: "desktop1604-test"}

we don't have a need for specifying desktop1604-test, it is the default now.

::: taskcluster/ci/source-test/python.yml:176
(Diff revision 1)
> +        using: run-task
> +        command: >
> +            source /builds/worker/scripts/xvfb.sh &&
> +            start_xvfb '1600x1200x24' 0 &&
> +            cd /builds/worker/checkouts/gecko &&
> +            ./mach python-test --subsuite reftest

why do we need to start_xvfb, isn't that done in the desktop image?
Attachment #8907087 - Flags: review?(jmaher) → review+
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED
Comment on attachment 8907087 [details]
Bug 1392390 - Create a reftest selftest harness,

https://reviewboard.mozilla.org/r/178800/#review183968

Follow-up already filed in bug 1399155

> it owuld be nice to test more features in the .list files, a few come to mind:
> http
> skip-if
> fuzzy-if
> random-if
> # commented out tests
> include submanifests

This is all tested already by the reftest-sanity suite, so I don't think there's much value in adding a second integration test that does the same thing.

However, I do think there's value if we can somehow figure out how to unittest just the reftest manifest parsing portion of the harness. I don't know how easy that would be right now, but it is definitely something that would be possible with bug 1353461. In fact, having a place to add a test for this was the main reason I wanted to work on this selftest harness in the first place :).

So if it's ok with you, I'd like to drop this issue for now. But I'll definitely be adding a unit test for all this stuff as part of bug 1353461.

> we don't have a need for specifying desktop1604-test, it is the default now.

Good to know!

> why do we need to start_xvfb, isn't that done in the desktop image?

Good question, I'm not sure. I copied this from the mochitest-harness task, I'll do a try run without it and see if things still pass or not.. though I suspect xvfb is needed for Firefox to run.
Comment on attachment 8907087 [details]
Bug 1392390 - Create a reftest selftest harness,

https://reviewboard.mozilla.org/r/178800/#review183968

> Good to know!

Oh, actually 'docker-image' is a required key for source-test tasks, so this needs to be here.
Comment on attachment 8907087 [details]
Bug 1392390 - Create a reftest selftest harness,

https://reviewboard.mozilla.org/r/178800/#review183968

> Good question, I'm not sure. I copied this from the mochitest-harness task, I'll do a try run without it and see if things still pass or not.. though I suspect xvfb is needed for Firefox to run.

Yes, this fails without xvfb:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=caf01cb44bf1007ddab4b42b206e89788b568ca8

I think this happens because for desktop tests, xvfb is set up via a script that gets called as part of the test command. This is a "source-test" task and that script doesn't get invoked by default for these kinds of tasks.
I think this is good to land. I did a bunch of retriggers in this try push and no sign of intermittents:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=93af92c2cf6c793d7da5112f1742f8c164a318d3
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/832cd9477043
Refactor common code out of mochitest selftests and into a new moztest.selftest module, r=jmaher
https://hg.mozilla.org/integration/autoland/rev/fcf7e964aba5
Create a reftest selftest harness, r=jmaher
https://hg.mozilla.org/mozilla-central/rev/832cd9477043
https://hg.mozilla.org/mozilla-central/rev/fcf7e964aba5
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.