✅ Skip borked on= tests for IE11 #30407
Conversation
| @@ -17,7 +17,7 @@ | |||
| import {AmpEvents} from '../../src/amp-events'; | |||
| import {createFixtureIframe, poll} from '../../testing/iframe.js'; | |||
|
|
|||
| describe('on="..."', () => { | |||
| describe.configure().run('on="..."', () => { | |||
There was a problem hiding this comment.
Per my understanding, changing describe( to describe.configure().run( is a no-op, since there's nothing between the configure() and the run(. Also, since IE tests are opt-in, a plain describe( block will not run on IE.
Is this different from what you're seeing?
There was a problem hiding this comment.
describe by itself appears to just be the Mocha global. __init_tests.js is what adds describe.configure, which actually instantiates the custom TestConfig we use, and it's within that constructor that IE is skipped by default. That's why adding it here causes the test to skip:
Here:
Elsewhere:
There was a problem hiding this comment.
Note that above, you can see A) 21 tests are executed (not 22, which included the one that failed) B) the failure caused all other tests in that file not to register, whereas now it counts them as skipped (ie. 466 vs 469)
There was a problem hiding this comment.
Interesting. Thanks for the clarification. Another reason in favor of removing legacy describe tests (#24144).
| @@ -17,7 +17,7 @@ | |||
| import {AmpEvents} from '../../src/amp-events'; | |||
| import {createFixtureIframe, poll} from '../../testing/iframe.js'; | |||
|
|
|||
| describe('on="..."', () => { | |||
| describe.configure().run('on="..."', () => { | |||
There was a problem hiding this comment.
Interesting. Thanks for the clarification. Another reason in favor of removing legacy describe tests (#24144).
After lots of digging, I can't track down the source of this issue. The error message is cryptic:
When enabling logging and digging deeper, the root seems to come from a
SyntaxErrorwithin an unhandled promise rejection, but it's been impossible to determine the source of theSyntaxErroritself. Further digging might yield useful information, but for know this test should just be skipped until a fix is indentified. (See #30372 for the long, painful slog attempting to log something of use to squash this issue)Part of broad effort under #28152 to enable IE11 integration tests as blocking