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

testharness.js: Calling setup() and promise_setup() after tests are created should be a harness error #20336

Open zcorpan opened 4 years ago

zcorpan commented 4 years ago

From https://github.com/web-platform-tests/wpt/pull/20187#issuecomment-555187502

I said:

@jugglinmike I think it seems weird to allow setup after tests have been created. Maybe we should make that a harness error?

@jugglinmike responded:

@zcorpan It's true that after tests have been created, setup doesn't cause any exceptions or harness errors. However, it's not exactly "allowed," either. The harness silently ignores such invocations:

https://github.com/web-platform-tests/wpt/blob/295f22bfeb7e6db748e6b0e28c31adfeb498b55d/resources/testharness.js#L2478-L2482

This condition seems like evidence of mistake in test design, so I agree that alerting authors via a harness error would be preferable. However, I would rather maintain parity with setup for this patch.

We could discuss implementing a harness error separately, either as a precursor to this change or as a follow-up.

I think we should make it a harness error, but this doesn't need to block promise_setup() I think.

stephenmcgruer commented 4 years ago

Marking roadmap due to inactivity (and lack of concern about inactivity)