search

web-platform-tests / wpt

Test suites for Web platform specs — including WHATWG, W3C, and others
https://web-platform-tests.org/
Other
5k stars 3.1k forks source link

Unify `./wpt check-stability` and `./wpt run --verify` #13406

Open foolip opened 6 years ago

foolip commented 6 years ago

Previous issues and PRs (now closed/merged) on the topic of stability checking:

However, with --stability gone, we're still left with two different things. On Travis we use ./wpt check-stability: https://github.com/web-platform-tests/wpt/blob/654dc43664f0ff6b84609d334cb05147c0594fbd/tools/ci/ci_stability.sh#L12

And on Taskcluster we use ./wpt run --verify: https://github.com/web-platform-tests/wpt/blob/207fa91e8016fcb650e4869e0da6ef036d7671b3/.taskcluster.yml#L100

It would be great with just a single approach that we can use everywhere.

@gsnedders @jgraham @jugglinmike

jugglinmike commented 6 years ago

This is more than just interface design. The semantics of the two commandsdiffer in a way that influences what passes for "stable". More on this here:

https://github.com/web-platform-tests/wpt/pull/12657#issuecomment-418220606

foolip commented 6 years ago

@jugglinmike thanks for that summary! Beyond the AABB vs. ABAB ordering, you can from the Taskcluster logs that there are also differences in restarts. Example from https://github.com/web-platform-tests/wpt/pull/13405:

::: ::: Running test verification step "Running tests in a loop 10 times"... :::

All results

/webstorage/event_initstorageevent.html

Subtest Results Messages
OK
initStorageEvent with 0 arguments FAIL assert_throws: function "() => event.initStorageEvent()" did not throw
initStorageEvent with 1 argument FAIL assert_equals: event.key expected (object) null but got (string) "undefined"
initStorageEvent with 8 sensible arguments PASS
initStorageEvent with 8 null arguments FAIL assert_equals: event.key expected (object) null but got (string) "null"
initStorageEvent with 8 undefined arguments FAIL assert_equals: event.key expected (object) null but got (string) "undefined"

::: ::: Running test verification step "Running tests in a loop with restarts 5 times"... :::

All results

/webstorage/event_initstorageevent.html

Subtest Results Messages
OK
initStorageEvent with 0 arguments FAIL assert_throws: function "() => event.initStorageEvent()" did not throw
initStorageEvent with 1 argument FAIL assert_equals: event.key expected (object) null but got (string) "undefined"
initStorageEvent with 8 sensible arguments PASS
initStorageEvent with 8 null arguments FAIL assert_equals: event.key expected (object) null but got (string) "null"
initStorageEvent with 8 undefined arguments FAIL assert_equals: event.key expected (object) null but got (string) "undefined"

::: Running tests in a loop 10 times : PASS ::: Running tests in a loop with restarts 5 times : PASS ::: ::: Test verification PASS :::

Anyway, it seems to me that --verify is the more featureful variant. @jgraham, after we've turned off Travis stability checking in favor of Taskcluster, is there any reason to keep check-stability around as well?

jgraham commented 6 years ago

No.

foolip commented 6 years ago

Alright! @jugglinmike, how are things looking for making Taskcluster stability task failures blocking?

mdittmer commented 5 years ago

Ping from your friendly neighbourhood ecosystem infra rotation

If this is priority:roadmap should it have an assignee?

foolip commented 5 years ago

I'll downgrade the priority. Once we've gotten rid of stability jobs from Travis entirely we can do this.