Increase timeout of amp-pixel integration test to address its flakiness. #19723
Conversation
| @@ -20,7 +20,7 @@ import {Services} from '../../src/services'; | |||
| import {createElementWithAttributes} from '../../src/dom'; | |||
|
|
|||
| describe.configure().skipIfPropertiesObfuscated().run('amp-pixel', function() { | |||
| this.timeout(1000); | |||
| this.timeout(15000); | |||
There was a problem hiding this comment.
This is a 15x increase in the potential running time for this test. Is it actually necessary? If many tests were to do the same thing, we could see a lot more overall timeouts on Sauce Labs / Travis.
Alternatively, is there a way to trigger the behavior being tested a lot faster when the runtime knows that tests are being run?
There was a problem hiding this comment.
there is no trigger. seems to me it's just the slowness in Edge. it is just a timeout anyway, for successful run, it normally should take less than 1s. The impact here is that it will fail 15X slower if it gotta fail, which I think is fine for Travis. I'm fine with changing this number to 5s if it passes tests on Edge. this 15s number is copied from other integration tests.
However, we do want it fail faster in local dev to save developer's time. That can be a separate improvement.
Anyway, this PR couldn't be merged alone, @choumx you might apply this change to your fixing PR.
For #19722