Make AMP tests less flaky

[rsimha]

The number of test failures / flakes during PR and push builds on Travis due to dirty global state appears to have increased over the recent weeks. It's time to explore ideas that will reduce flakiness by isolating tests from one another and eliminating dirty global state.

Some ideas:

• Update beforeEach and afterEach in test/_init_tests.js to be more thorough with how they clear state between tests
• Prevent tests from performing operations that modify global state
• Fail tests that when errors are logged by the runtime (unless they're expected) (🏗 Surface console errors during tests as mocha failures #14336)
• Create a new window object (with a new this) for each test (inefficient? infeasible?)

(More ideas welcome via comments below)

[alanorozco]

1. Stub experiment toggles by default

Some sort of test-time replacement of isExperimentOn and toggleExperiment so that they use stubs instead of affecting the window state. Tests that need to validate the behavior of these functions can be whitelisted.

2. Forbid experiment toggles in test

Disadvantage: tests need to be rewritten to stub a static method that checks for the experiment.

3. Force all services to be stubbed in tests

Services are likely to affect global state since they're singletons. Implement some mechanism (similar to above) to either:

1. automatically mock services or
2. forbid the direct use of services without mocks

This would also reduce the running time of a lot of tests.

[dreamofabear]

AKA return of the prodigal son: go/amp-test-excellence

/cc @lannka 😂

[rsimha]

It turns out that several AMP tests do asynchronous stuff (like throwing errors), but don't correctly use the done() callback to indicate that the asynchronous tasks have completed.

This is being tracked by #15658

[dreamofabear]

FR: Run all tests in the scope of a child iframe such that window, document and other globals don't reference the test runner's window.

Motivation: #16586

[rsimha]

Reopening, since we still haven't solved the global window / document issue.

[lannka]

Did we explore Karma option, how possible it is to open separate browser tab for each test file (not each test case).

Another hacky idea regarding to the async code leakage (I don't quite like, but shared anyway):

@rsimha we've talked a bit that if we can wait a full tick at end of each test to make sure async executions not leaked to next test (those execution could potentially throw and unpredictably fail unrelated test).

We couldn't have it done in afterEach because for some reason thrown in beforeEach or afterEach would sometimes be swallowed by Mocha silently and stop all the remaining tests.

I'm wondering if we can hack/wrap the it method to do the job.

[ampprojectbot]

This issue hasn't been updated in awhile. @rsimha Do you have any updates?

[ampprojectbot]

This issue hasn't been updated in awhile. @rsimha Do you have any updates?

[rsimha]

Since this was filed, several things have been done to improve test flakiness (see PRs linked above). However, more remains to be done in the areas of improving transitions during tests (#9164), fixing incorrect use of test assertions (#28128), and preventing external network requests (#26756). Keeping this uber-issue around until they are done.

[rsimha]

[Infra triage] Assigning this uber-issue to @rileyajones for visibility. It may no longer be useful to keep this issue around since most ideas have already been split off into separate efforts. Feel free to close if this is the case.

[rsimha]

Closing this now that all AMP unit and integration tests are sandboxed by describes. Issues related global state will be tracked by #23837.