Amphtml: Various `amp-list` tests are timing out on Travis (local Chrome)

Created on 25 Apr 2019 · 10Comments · Source: ampproject/amphtml

Examples from the past couple of hours:

https://travis-ci.org/ampproject/amphtml/jobs/524654095#L1533
https://travis-ci.org/ampproject/amphtml/jobs/524510658#L1533
https://travis-ci.org/ampproject/amphtml/jobs/524244291#L1533

DESCRIBE => amp-list (integration)
  DESCRIBE => [is-layout-container]
    DESCRIBE =>  
      IT => should change to layout container on bind
        ✗ Error: Timeout waiting for "amp-list" to layout
            at poll (/tmp/testing/iframe.js:425:18)
HeadlessChrome 74.0.3729 (Linux 0.0.0) amp-list (integration) [is-layout-container]   should change to layout container on bind FAILED
    Error: Timeout waiting for "amp-list" to layout
        at poll (/tmp/testing/iframe.js:425:18)

DESCRIBE => amp-bind
  DESCRIBE => + amp-list, amp-mustache:0.2
    DESCRIBE =>  
      IT => evaluate bindings in children
        ✗ Error: Timeout waiting for "amp-list" to layout
            at poll (/tmp/testing/iframe.js:425:18)
HeadlessChrome 74.0.3729 (Linux 0.0.0) amp-bind + amp-list, amp-mustache:0.2   evaluate bindings in children FAILED
    Error: Timeout waiting for "amp-list" to layout
        at poll (/tmp/testing/iframe.js:425:18)

This is causing redness on master.

High Priority Flaky Tests Bug

Source

rsimha

All 10 comments

/to @jridgewell @choumx to triage

rsimha on 25 Apr 2019

This is my fault. I'll take a look.

cathyxz on 26 Apr 2019

👍1

Thanks!

jridgewell on 26 Apr 2019

Sigh. I added in some test code here the last time these tests flaked to figure out where the timeouts were happening. It looks like it literally times out waiting for amp-list to layout for the first time.

  yield browser.waitForElementLayout('amp-list', TIMEOUT);

Basically, what I'm trying test here is to assert that it changes to container layout correctly if I click on a button after <amp-list> finishes layout. If we can't get the amp-list to layout, there really isn't a whole lot of point in testing anything at all. The last time I troubleshooted this test, this timeout didn't repro locally, ever (it finishes in under 20ms usually, and 300ms max, FF, Safari, Chrome, Chrome Headless on macOS which is Linux). We can of course skip these tests for now, but I think there's an open question of why something that finishes in < 300ms takes > 10s on Travis. Otherwise I don't really know how we can write or run integration tests at all.... Are ~particular environments~ Headless Chrome on Linux (noted the same env from all three failing examples) just extra slow on Travis?

cathyxz on 26 Apr 2019

Thanks for the context @cathyxz! In general, it's true that Travis VMs are slower than our development workstations / laptops. Therefore, tests that rely on complex / asynchronous stuff tend to take longer on Travis, and are often exposed as timeouts.

rsimha on 26 Apr 2019

So two different options to fix this:

Rewrite this test as a unit test asserting the container layout behavior and an e2e test asserting the e2e behavior.
Increase the timeout.
Philosophically speaking, @rsimha are you cool with E2E tests being the tests that are designed to have long timeouts / be relatively flaky?

cathyxz on 26 Apr 2019

If your test can run in isolation with just component code, but without the compiled runtime, write a unit test.
If it tests interactions between components, write an integration test.
If it relies on external services / browser interactions, write an E2E test.

Based on this, I'd say E2E tests are the way to go, although we'd like to avoid flakiness!

/cc @cvializ @estherkim for advice on E2E tests.

rsimha on 26 Apr 2019

I like option 1 where we split the layout behavior check into a unit test. It looks like it'd be straightforward through calling mutatedAttributesCallback.

My philosophy for E2E tests is that they should test for things that the user would see through interactions. So changing the layout from size-defined to container is not something that the user would observe on their own, so a non-e2e test should verify that behavior. Something like seeing new text on the screen or anything else user-visible would be good to verify in an e2e test.