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

Document how to update infrastructure/metadata #12697

Open
gsnedders opened this issue Aug 27, 2018 · 6 comments
Open

Document how to update infrastructure/metadata #12697

gsnedders opened this issue Aug 27, 2018 · 6 comments
Assignees
Labels

Comments

@gsnedders
Copy link
Member

Having hacked around #12696, running ./wpt update-expectations --product safari --metadata infrastructure/metadata/ --manifest MANIFEST.json safari.log I get:

diff --git a/infrastructure/metadata/infrastructure/expected-fail/failing-test.html.ini b/infrastructure/metadata/infrastructure/expected-fail/failing-test.html.ini
index b954a0e9b7..7fad049fca 100644
--- a/infrastructure/metadata/infrastructure/expected-fail/failing-test.html.ini
+++ b/infrastructure/metadata/infrastructure/expected-fail/failing-test.html.ini
@@ -1,4 +1,5 @@
 [failing-test.html]
+  expected: ERROR
   [Failing test]
     expected: FAIL
 
diff --git a/infrastructure/metadata/infrastructure/expected-fail/timeout.html.ini b/infrastructure/metadata/infrastructure/expected-fail/timeout.html.ini
index 53b281f835..5f8f2d8063 100644
--- a/infrastructure/metadata/infrastructure/expected-fail/timeout.html.ini
+++ b/infrastructure/metadata/infrastructure/expected-fail/timeout.html.ini
@@ -1,4 +1,5 @@
 [timeout.html]
-  expected: TIMEOUT
+  expected: ERROR
   [Test that should time out]
     expected: NOTRUN
+

This isn't what I expect. How do I update expectations for Safari only, without overwriting existing expectations (note I didn't pass in --ignore-existing)?

If I instead run it without --product, which is apparently optional, like $ ./wpt update-expectations --metadata infrastructure/metadata/ --manifest MANIFEST.json safari.log firefox.log chrome.log, I get:

Processing log 1/3
Processing log 2/3
Processing log 3/3
Traceback (most recent call last):
  File "./wpt", line 5, in <module>
    wpt.main()
  File "/Users/gsnedders/Documents/other-projects/wpt/web-platform-tests/tools/wpt/wpt.py", line 132, in main
    rv = script(*args, **kwargs)
  File "/Users/gsnedders/Documents/other-projects/wpt/web-platform-tests/tools/wpt/update.py", line 32, in update_expectations
    updater.run()
  File "/Users/gsnedders/Documents/other-projects/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/update.py", line 150, in run
    rv = update_runner.run()
  File "/Users/gsnedders/Documents/other-projects/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/gsnedders/Documents/other-projects/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/base.py", line 32, in run
    self.create(state)
  File "/Users/gsnedders/Documents/other-projects/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/update.py", line 96, in create
    runner.run()
  File "/Users/gsnedders/Documents/other-projects/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/gsnedders/Documents/other-projects/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/base.py", line 32, in run
    self.create(state)
  File "/Users/gsnedders/Documents/other-projects/wpt/web-platform-tests/tools/wptrunner/wptrunner/update/metadata.py", line 35, in create
    stability=state.stability)
  File "/Users/gsnedders/Documents/other-projects/wpt/web-platform-tests/tools/wptrunner/wptrunner/metadata.py", line 45, in update_expected
    stability=stability):
  File "/Users/gsnedders/Documents/other-projects/wpt/web-platform-tests/tools/wptrunner/wptrunner/metadata.py", line 201, in update_from_logs
    for item in update_results(id_test_map, property_order, boolean_properties, stability):
  File "/Users/gsnedders/Documents/other-projects/wpt/web-platform-tests/tools/wptrunner/wptrunner/metadata.py", line 217, in update_results
    default_expected_by_type)
  File "/Users/gsnedders/Documents/other-projects/wpt/web-platform-tests/tools/wptrunner/wptrunner/metadata.py", line 575, in update
    test.coalesce_properties(stability=stability)
  File "/Users/gsnedders/Documents/other-projects/wpt/web-platform-tests/tools/wptrunner/wptrunner/manifestupdate.py", line 245, in coalesce_properties
    prop_update.coalesce(stability)
  File "/Users/gsnedders/Documents/other-projects/wpt/web-platform-tests/tools/wptrunner/wptrunner/manifestupdate.py", line 358, in coalesce
    self.add_new(unconditional_value, stability)
  File "/Users/gsnedders/Documents/other-projects/wpt/web-platform-tests/tools/wptrunner/wptrunner/manifestupdate.py", line 422, in add_new
    boolean_properties=self.node.root.boolean_properties)
  File "/Users/gsnedders/Documents/other-projects/wpt/web-platform-tests/tools/wptrunner/wptrunner/manifestupdate.py", line 585, in group_conditionals
    prop_set = tuple((prop, run_info[prop]) for prop in include_props)
  File "/Users/gsnedders/Documents/other-projects/wpt/web-platform-tests/tools/wptrunner/wptrunner/manifestupdate.py", line 585, in <genexpr>
    prop_set = tuple((prop, run_info[prop]) for prop in include_props)
KeyError: 'e10s'

How do you update expectations for all browsers? I remember previously you passed in all the logs at once, but this seems not to work.

@gsnedders
Copy link
Member Author

See https://gist.github.com/gsnedders/fadcc35d961b9958a2a4805fc194d03d for the logs I was using.

@jgraham
Copy link
Contributor

jgraham commented Aug 30, 2018

It doesn't know what the total space of all configurations is, so if you pass it one set of logs it assumes they represent all possible results for those tests. It is possible to do the opposite and assume that the output should be as specific as possible given the input, but that's not implemented.

I think --product defaults to firefox so if you don't supply one that's what you get. It could perhaps just be mandatory.

I think updating the logs for multiple products at once might just be broken because the product is supplied per invocation via the command line, but things that can be different need to be supplied as part of the logs themselves. So probably the first step to making that possible is to start logging the product as part of the run_info.

@foolip
Copy link
Member

foolip commented Oct 5, 2018

I found my way here while playing with foolip#5, by googling for "wpt update-expectations" :)

For infrastructure/metadata we'll need product-specific expectations, so unless the script would need all expectations as its input I presume.

@Hexcles it may interest you to have a look at this analogue to Chromium's expectations and baselines.

@foolip
Copy link
Member

foolip commented Oct 5, 2018

FWIW, the first thing I looked for was something like ./wpt run --update-expectations, since Chromium has run_web_tests.py --reset-expectations. @jgraham would that be complicated to support, even if it doesn't solve the problem of merging expectations?

@jgraham
Copy link
Contributor

jgraham commented Oct 17, 2019

Making wpt run take --update-expectations seems weird. I think the way this works for Chromium is likely different since that (aiui) has a single expectation file per platform, whereas we have one per test covering all platforms. So you can't (typically) update the expectations from a single run, but need to update using the logs from the full set of platforms (unless you happen to know that all platforms share the same result). The updater tries to do something reasonable with partial logs (preserving conditions that can't be reached by supplied logs), but it's hard to do right in general.

I think the central problem here is that we are hardcoding the list of update properties in the browser files. But that's not right; the list of properties to use when updating is a function of the CI system rather than a function of the browser. For the GitHUb infrastructure we have multiple products but only a single platform per product. For Gecko CI we have one product but many different platforms and configurations within those platforms. So I think the right fix for this is probably to move that configuration out into the frontend, so it's useful outside the Gecko context.

@foolip
Copy link
Member

foolip commented Oct 17, 2019

Maintaining expectations when we have different CI systems, browsers and platform involved definitely has its complications, and if there's something in Gecko that could be moved to WPT to help manage that, that'd be great.

However, this issue was filed about the tooling that does exist not working at all, and throwing an exception. I could reproduce it at the time and haven't tried using it since, instead writing .ini files by hand even when generating them from a single set of results would have worked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants