Open Bug 1286900 Opened 8 years ago Updated 2 years ago

Run wpt tests from source checkout

Categories

(Testing :: web-platform-tests, defect, P3)

Product:
Testing
Component:
web-platform-tests
Type:
defect

Tracking

(firefox52 wontfix)

Tracking Flags:
Tracking Status
firefox52 --- wontfix

People

(Reporter: gps, Unassigned)

References

(Blocks 3 open bugs)

Details

Attachments

(6 files)

Bug 1286900 - Ensure WPT tests have a version control checkout;
58 bytes, text/x-review-board-request
ted
: review+
Details
Bug 1286900 - Reformat WPT command argument additions;
58 bytes, text/x-review-board-request
ted
: review+
Details
Bug 1286900 - Inline WPT arguments into script;
58 bytes, text/x-review-board-request
ted
: review+
Details
Bug 1286900 - Use WPT files from source checkout;
58 bytes, text/x-review-board-request
ted
: review+
Details
Bug 1286900 - Stop producing web-platform tests zip file;
58 bytes, text/x-review-board-request
ted
: review+
Details
Bug 1286900 - Add environment variable to define location of mozinfo.json;
58 bytes, text/x-review-board-request
ted
: review+
Details 
We believe that running WPT tests from a Mercurial checkout will be faster than from test archives.

This bug is about testing that theory.
Specifically, we'd have the mozharness script for the test job clone the Mercurial repository, download the build as usual, possibly download the tests.common.zip (if WPT needs any of the binaries from there), and run the test command directly from the source directory (perhaps even using ./mach test web-platform-tests).

We might need to either fix `mach test` to do a minimal configure or have the build script run `mach configure` first, presumably with `--disable-compile-environment` so it's fast.
certutil is required to set up the profile. I don't recall any other tests.common.zip requirements. If we use |mach web-platform-tests| directly (which I think would be good), we'll need to allow the certutil and binary to be in a different place compared to a local build.
Blocks: thunder-try
Depends on: 1291365
Depends on: 1294234
Depends on: 1295380
Depends on: 1296397
Depends on: 1302907
Depends on: 1304176
Assignee: nobody → gps
Status: NEW → ASSIGNED
Comment on attachment 8793159 [details]
Bug 1286900 - Ensure WPT tests have a version control checkout;

https://reviewboard.mozilla.org/r/79954/#review79454

So in buildbot we'll bootstrap from mozharness then do a separate hg clone to get the full source repo? Not great, but if it's better in Taskcluster it's probably not worth over-optimizing.

::: testing/mozharness/mozharness/mozilla/testing/testbase.py:137
(Diff revision 3)
> +            dirs['checkout'] = os.path.join(dirs['base_work_dir'], 'checkout')
> +
> +        if 'HG_SHARE_BASE_DIR' in os.environ:
> +            dirs['hg_shared'] = os.environ['HG_SHARE_BASE_DIR']
> +        else:
> +            dirs['hg_shared'] = os.path.join(dirs['base_work_dir'], 'hg-shared')

I assume all of this is consumed by VCSMixin? It'd be nice to mention that. (I always have a hard time keeping track of what code does what in mozharness with all the mixins.)
Attachment #8793159 - Flags: review?(ted) → review+
Comment on attachment 8793160 [details]
Bug 1286900 - Reformat WPT command argument additions;

https://reviewboard.mozilla.org/r/79956/#review79456
Attachment #8793160 - Flags: review?(ted) → review+
Comment on attachment 8793161 [details]
Bug 1286900 - Inline WPT arguments into script;

https://reviewboard.mozilla.org/r/79958/#review79458

We are really good at giving ourselves way too many options, aren't we?
Attachment #8793161 - Flags: review?(ted) → review+
Comment on attachment 8793162 [details]
Bug 1286900 - Use WPT files from source checkout;

https://reviewboard.mozilla.org/r/79960/#review79460
Attachment #8793162 - Flags: review?(ted) → review+
Comment on attachment 8793163 [details]
Bug 1286900 - Stop producing web-platform tests zip file;

https://reviewboard.mozilla.org/r/79962/#review79466

\o/ one down, a few more to go!
Attachment #8793163 - Flags: review?(ted) → review+
This bug is good as-is, maybe we should file a followup to run the tests directly from the mach command? That ought to let us remove a whole bunch of stuff from the mozharness invocation.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #23)
> This bug is good as-is, maybe we should file a followup to run the tests
> directly from the mach command? That ought to let us remove a whole bunch of
> stuff from the mozharness invocation.

Yes, that is very tempting. There are a number of mozharness scripts that IMO add very little value, especially now that we can do source checkouts easily. It's at least worth having the bug on file.
I have to hold off landing this because there's a few lingering failures on Try. I suspect some files aren't in the locations the tests expect them to be in. I may have to poke jgraham for help on Monday...
Blocks: 1305205
Comment on attachment 8793162 [details]
Bug 1286900 - Use WPT files from source checkout;

Needs re-review for mozinfo.json changes.
Attachment #8793162 - Flags: review+ → review?(ted)
Comment on attachment 8795070 [details]
Bug 1286900 - Add environment variable to define location of mozinfo.json;

https://reviewboard.mozilla.org/r/81250/#review79910

This makes sense. We've been relying on the crappy "package mozinfo.json next to the main .py file" behavior since we started using it.

::: testing/mozbase/mozinfo/mozinfo/mozinfo.py:210
(Diff revision 1)
> -                 searched after first looking in the root of the objdir
> -                 if the current script is being run from a Mozilla objdir.
> +
> +    1) MOZINFO_PATH environment variable.
> +    2) objdir (via a build system context)
> +    3) Traversing directories ``dirs`` for a mozinfo.json file.
>  
>      Returns the full path to mozinfo.json if it was found, or None otherwise.

Did you also want to change this to raise if it can't find mozinfo.json? I don't know why we wrote it this way originally, but I suspect none of the test harnesses that use it are going to do the right thing if they are run without a mozinfo.json present.
Attachment #8795070 - Flags: review?(ted) → review+
Comment on attachment 8795070 [details]
Bug 1286900 - Add environment variable to define location of mozinfo.json;

https://reviewboard.mozilla.org/r/81250/#review79910

> Did you also want to change this to raise if it can't find mozinfo.json? I don't know why we wrote it this way originally, but I suspect none of the test harnesses that use it are going to do the right thing if they are run without a mozinfo.json present.

I think that would break running things locally? I'm sure it would break running wpt outside mozilla-* at least.
Comment on attachment 8795070 [details]
Bug 1286900 - Add environment variable to define location of mozinfo.json;

https://reviewboard.mozilla.org/r/81250/#review79910

> I think that would break running things locally? I'm sure it would break running wpt outside mozilla-* at least.

I was tempted to make this raise. But I figured it would be best deferred to a follow-up, as I didn't want to scope bloat and risk backout.

I made the new environment code fail fast because it isn't subject to backwards compatible behavior.
Comment on attachment 8795070 [details]
Bug 1286900 - Add environment variable to define location of mozinfo.json;

https://reviewboard.mozilla.org/r/81250/#review79910

> I was tempted to make this raise. But I figured it would be best deferred to a follow-up, as I didn't want to scope bloat and risk backout.
> 
> I made the new environment code fail fast because it isn't subject to backwards compatible behavior.

In a local build we always have a mozinfo.json in the objdir. Does wpt work with Firefox outside of a mozilla repo if it doesn't have mozinfo? Surely all the manifest conditionals would be missing the proper info?
Comment on attachment 8793162 [details]
Bug 1286900 - Use WPT files from source checkout;

https://reviewboard.mozilla.org/r/79960/#review80000
Attachment #8793162 - Flags: review?(ted) → review+
Comment on attachment 8793159 [details]
Bug 1286900 - Ensure WPT tests have a version control checkout;

https://reviewboard.mozilla.org/r/79954/#review79454

> I assume all of this is consumed by VCSMixin? It'd be nice to mention that. (I always have a hard time keeping track of what code does what in mozharness with all the mixins.)

Yes, VCSMixin does all the magic.

At some point I want to start extracting the useful methods of mixins to functions so the code is more obvious to read and can be reused in other contexts (such as mach commands).
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6b5f059a85ad
Add environment variable to define location of mozinfo.json; r=ted
https://hg.mozilla.org/integration/autoland/rev/01c3527edde5
Ensure WPT tests have a version control checkout; r=ted
https://hg.mozilla.org/integration/autoland/rev/5b0f69c86028
Reformat WPT command argument additions; r=ted
https://hg.mozilla.org/integration/autoland/rev/3928e945b584
Inline WPT arguments into script; r=ted
https://hg.mozilla.org/integration/autoland/rev/4b08ed30e0a5
Use WPT files from source checkout; r=ted
https://hg.mozilla.org/integration/autoland/rev/859457467e3c
Stop producing web-platform tests zip file; r=ted
Comment on attachment 8795070 [details]
Bug 1286900 - Add environment variable to define location of mozinfo.json;

https://reviewboard.mozilla.org/r/81250/#review79910

> In a local build we always have a mozinfo.json in the objdir. Does wpt work with Firefox outside of a mozilla repo if it doesn't have mozinfo? Surely all the manifest conditionals would be missing the proper info?

It works in the sense that it runs, and that's a supported use case. It is true that not all the expectation data will work (although most of it will because most does not depend on the extra build-added mozinfo fields, as you see from the fact that not everything failed before this was added to the patch).
That seems like a strange and perilous thing! I guess ideally the harness would just error if you tried to use something in a conditional that wasn't defined.
Blocks: 1305795
Backed out in https://hg.mozilla.org/integration/autoland/rev/aed585286446d6d39996d8c878a6ccabdef0d4f5 for needing who knows how many dependencies including but not limited to bug 1305877 to make it be a win, rather than a loss which runs the worst chunk over maxtime on Windows debug when it doesn't just hang while cloning or just pulling from mozilla-unified.
Depends on: 1305903
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6ed46e8ac86c
Add environment variable to define location of mozinfo.json; r=ted
https://hg.mozilla.org/integration/autoland/rev/43df1e962f8e
Ensure WPT tests have a version control checkout; r=ted
https://hg.mozilla.org/integration/autoland/rev/2de97e3cfcb3
Reformat WPT command argument additions; r=ted
https://hg.mozilla.org/integration/autoland/rev/a1311218621b
Inline WPT arguments into script; r=ted
https://hg.mozilla.org/integration/autoland/rev/bde587e47d00
Use WPT files from source checkout; r=ted
https://hg.mozilla.org/integration/autoland/rev/b4844ee1e542
Stop producing web-platform tests zip file; r=ted
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/748abfe6748e
Run Mercurial with PYTHONUNBUFFERED=1; r=me
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3fff88de2f2d
Set env explicitly because mozharness; r=me
As philor points out, this was causing timeouts on win7vm wpt tests, so all of this backed out in https://hg.mozilla.org/mozilla-central/rev/cda7c3e91ad7eb8119e1ae352cf362d9afa19918

https://treeherder.mozilla.org/logviewer.html#?job_id=4451408&repo=autoland
Status: RESOLVED → REOPENED
status-firefox52: fixed → affected
Flags: needinfo?(gps)
Resolution: FIXED → ---
Target Milestone: mozilla52 → ---
Depends on: 1307798
This is blocked on EBS performance on Windows testers. I don't need the needinfo set. I'll reland this as soon as it is unblocked.
Status: REOPENED → ASSIGNED
Flags: needinfo?(gps)
So how does the wontfix in bug 1307798 affect this? I am blocking on this change for some of the wpt stability linting that I'd like to try.
... good question. Needinfo me to follow up on this.
Flags: needinfo?(gps)
I'm not sure what we can do here. We may just have to deal with whatever performance issues this presents.

Since I'll be away for ~1 month, I recommend following up with Ted. I /think/ the old commits should rebase pretty cleanly. Hopefully it isn't much work to flip the switch if we decide to move forward.

As long as automation is using `hg robustclone` and caches are configured properly, there shouldn't be a capacity problem for hg.mozilla.org. But it is something to watch if we enable this.
Flags: needinfo?(gps)
Too late for firefox 52, mass-wontfix.
status-firefox52: affected → wontfix
Blocks: 1375499
Depends on: 1390700
Blocks: 1432287
Moving the tracker bugs to bug 1432287.
No longer blocks: fastci, thunder-try

The bug assignee didn't login in Bugzilla in the last 7 months.
:jgraham, could you have a look please?
For more information, please visit auto_nag documentation.

Assignee: gps → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(james)

I'm not aware of any current plans to revive the work to move to a source checkout.

Severity: normal → S4
Flags: needinfo?(james)
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: