Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run infrastructure tests every times tools change #9676

Merged
merged 5 commits into from Feb 28, 2018

Conversation

Projects
None yet
6 participants
@jgraham
Copy link
Contributor

jgraham commented Feb 26, 2018

This change is Reviewable

@jgraham jgraham requested a review from gsnedders Feb 26, 2018

@wpt-pr-bot wpt-pr-bot added the infra label Feb 26, 2018

[html-elements.html]
[Compare CSS div definitions (only valid if pre-reqs pass)]
expected:
if product == "chrome": PASS

This comment has been minimized.

Copy link
@gsnedders

gsnedders Feb 26, 2018

Contributor

Are you running this on master? On master, after #9674, only Firefox should pass!

@gsnedders

This comment has been minimized.

Copy link
Contributor

gsnedders commented Feb 26, 2018

First commit should be separating "metadata and manifest" not "meanifest". Also, that fixes #7959.

@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Feb 26, 2018

Build PASSED

Started: 2018-02-28 22:27:54
Finished: 2018-02-28 22:32:16

View more information about this build on:

@jgraham jgraham force-pushed the infratest branch 5 times, most recently from 3b4b721 to 00bc577 Feb 26, 2018

@jgraham

This comment has been minimized.

Copy link
Contributor Author

jgraham commented Feb 27, 2018

Also fixes #9543

@gsnedders
Copy link
Contributor

gsnedders left a comment

Can you also rebase this on master in case #9602 changed anything?

@@ -430,8 +431,7 @@ def update_manifest(self, manifest_path, tests_path, url_base="/",

manifest.write(manifest_file, manifest_path)

def load_manifest(self, tests_path, metadata_path, url_base="/"):
manifest_path = os.path.join(metadata_path, "MANIFEST.json")
def load_manifest(self, tests_path, manifest_path, url_base="/", **kwargs):

This comment has been minimized.

Copy link
@gsnedders

gsnedders Feb 28, 2018

Contributor

What's the point in changing this to accept **kwargs?

This comment has been minimized.

Copy link
@jgraham

jgraham Feb 28, 2018

Author Contributor

It's populated from the dictionary we build but only cares about some keys. Otherwise we would either need to change the caller or have some explicit unused parameters.

This comment has been minimized.

Copy link
@jdm

jdm Mar 3, 2018

Contributor

This change broke the Servo update process:

TypeError: load_manifest() takes at least 3 arguments (3 given)

  File "/Users/servo/buildbot/slave/mac-nightly/build/python/servo/testing_commands.py", line 453, in update_wpt
    return run_globals["update_tests"](**kwargs)
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/update.py", line 25, in update_tests
    rv = update.run_update(logger, **kwargs)
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/update/__init__.py", line 26, in run_update
    return updater.run()
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/update.py", line 151, in run
    rv = update_runner.run()
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/base.py", line 61, in run
    rv = step(self.logger).run(step_index, self.state)
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/base.py", line 32, in run
    self.create(state)
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/update.py", line 97, in create
    runner.run()
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/base.py", line 61, in run
    rv = step(self.logger).run(step_index, self.state)
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/base.py", line 32, in run
    self.create(state)
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/metadata.py", line 37, in create
    stability=state.stability)
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/web-platform-tests/tools/wptrunner/wptrunner/metadata.py", line 40, in update_expected
    manifests = load_test_manifests(serve_root, test_paths)
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/web-platform-tests/tools/wptrunner/wptrunner/metadata.py", line 27, in load_test_manifests
    return manifest_loader.load()
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/web-platform-tests/tools/wptrunner/wptrunner/testloader.py", line 394, in load
    **paths)

@jgraham jgraham force-pushed the infratest branch from 00bc577 to 0ec14a3 Feb 28, 2018

@gsnedders
Copy link
Contributor

gsnedders left a comment

you need to make you shell script executable and then Travis will pass, r+ with that

jgraham added some commits Feb 26, 2018

Enable separating the metadata and manifest paths.
The current wptrunner setup assumes that the MANIFEST.json file is
always at the root of the metadata path. But there are scenarios in
which this need not be true e.g. if the metadata is in-tree and the
manifest is not. So we separate these out by porviding an extra
command line argument for the manifest path, and corresponding key in
the config file. If this is not supplied the old value is assumed as a
default.
Allow supplying extra properties to use when updating metadata.
The properties are read from a run_info.json and may be used to
distinguish runs. Default properties are set on a per-product
basis, but in some scenarios we want additional properties to be
considered for the metadata e.g. when updaating multi-browser metadata
we want to consider the full product name.

It may be better in the long run to simply allow overriding the full
list of properties and always having the user (or context-specific
caller) supply this list rather than having any hardcoded defaults.
Add a wpt command for updating expectation metadata.
The |wpt update-expectations| command takes a list of raw log files,
and updates the expectation data located in the directory supplied
thorugh the --metadata keyword argument. This metadata is used by
subsequent test runs to determine the expected test results.

By default the "product" property is used as a possible conditional on
the data; this means that if multiple logs are passed in
simultaneously then different results will be distinguished by "if
product==foo" conditions in the metadata.
Add a wptrunner_infrastructure job to CI.
This runs all the tests under infrastructure/ every time that the
tools/ directory changes, as a smoketest to ensure that nothing is
badly broken.

@jgraham jgraham force-pushed the infratest branch from 0ec14a3 to 85f878b Feb 28, 2018

@jgraham jgraham merged commit b73c5d4 into master Feb 28, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@gsnedders gsnedders deleted the infratest branch Feb 28, 2018

@jdm

This comment has been minimized.

Copy link
Contributor

jdm commented Mar 1, 2018

This broke the automated WPT sync process for Servo:

TypeError: can only concatenate list (not "NoneType") to list

  File "/Users/servo/buildbot/slave/mac-nightly/build/python/servo/testing_commands.py", line 453, in update_wpt
    return run_globals["update_tests"](**kwargs)
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/update.py", line 25, in update_tests
    rv = update.run_update(logger, **kwargs)
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/update/__init__.py", line 26, in run_update
    return updater.run()
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/update.py", line 151, in run
    rv = update_runner.run()
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/base.py", line 61, in run
    rv = step(self.logger).run(step_index, self.state)
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/base.py", line 32, in run
    self.create(state)
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/update.py", line 97, in create
    runner.run()
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/base.py", line 61, in run
    rv = step(self.logger).run(step_index, self.state)
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/base.py", line 32, in run
    self.create(state)
  File "/Users/servo/buildbot/slave/mac-nightly/build/tests/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/metadata.py", line 14, in create
    state.property_order = property_order + state.extra_properties
@jdm

This comment has been minimized.

Copy link
Contributor

jdm commented Mar 1, 2018

How does f9debd2#diff-17385117fc368a39018ba00459e60b63R24 not cause similar problems since the default value is None?

@gsnedders

This comment has been minimized.

Copy link
Contributor

gsnedders commented Mar 2, 2018

I believe that issue was fixed by @jdm in #9746.

@foolip

This comment has been minimized.

Copy link
Contributor

foolip commented Jun 7, 2018

@jgraham, note this job can fail with no output for 2 minutes, happened here:
https://travis-ci.org/web-platform-tests/wpt/jobs/389148638

@gsnedders

This comment has been minimized.

Copy link
Contributor

gsnedders commented Jun 8, 2018

@foolip I presume that job was restarted, given it shows it passing.

@foolip

This comment has been minimized.

Copy link
Contributor

foolip commented Jun 11, 2018

@gsnedders, yes, I restarted the job, assuming that it'd get a new ID and the old link would continue to work :/

@gsnedders

This comment has been minimized.

Copy link
Contributor

gsnedders commented Jun 11, 2018

@foolip never been the case with Travis, sadly :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.