Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test/integration/test-visibility-states.js flaky on master #9757

Closed
rsimha opened this issue Jun 7, 2017 · 7 comments
Closed

test/integration/test-visibility-states.js flaky on master #9757

rsimha opened this issue Jun 7, 2017 · 7 comments

Comments

@rsimha
Copy link
Contributor

rsimha commented Jun 7, 2017

The integration test test/integration/test-visibility-states.js relies on uncompiled JS. However, after recent build optimizations, gulp dist only generates minified JS.

Figure out if test-visibility-states.js can be made to run with just gulp dist, and if not, figure out a minimal addition to gulp dist to create compiled and minified versions of those specific files the test needs.

@jridgewell
Copy link
Contributor

I'll need to revert #9758 after fixing.

@rsimha
Copy link
Contributor Author

rsimha commented Jun 8, 2017

Still red on master: https://travis-ci.org/ampproject/amphtml/jobs/240618920

@rsimha rsimha reopened this Jun 8, 2017
@jridgewell
Copy link
Contributor

Damn, my fault. I missed the private #setVisibilityState_ when reviewing it.

@zhouyx
Copy link
Contributor

zhouyx commented Jun 12, 2017

I still see test-visibility-states fail on master.
https://travis-ci.org/ampproject/amphtml/jobs/242105387
@jridgewell Could you take a look?

Mobile Safari 9.0.0 (iOS 9.1.0) Viewer Visibility State Element Transitions "before each" hook for "calls layout when going to VISIBLE" FAILED
timeout of 10000ms exceeded. Ensure the done() callback is being called in this test.
........................................
Mobile Safari 9.0.0 (iOS 9.1.0): Executed 119 of 216 (1 FAILED) (skipped 73) (1 min 2.153 secs / 36.182 secs)
..............................................................................
Mobile Safari 10.0.0 (iOS 10.0.0) Viewer Visibility State Element Transitions "before each" hook for "calls layout when going to VISIBLE" FAILED
timeout of 10000ms exceeded. Ensure the done() callback is being called in this test.

@zhouyx zhouyx reopened this Jun 12, 2017
@rsimha
Copy link
Contributor Author

rsimha commented Jun 12, 2017

Seems like there's a common theme of beforeEach() timing out in all these failures. Let me know if there's any infra work that can go into making these tests more resilient.

@rsimha rsimha changed the title test/integration/test-visibility-states.js fails on master because it relies on uncompiled JS test/integration/test-visibility-states.js flaky on master Jun 12, 2017
@zhouyx
Copy link
Contributor

zhouyx commented Jun 14, 2017

Oh no! still get integration tests failed 😭
https://travis-ci.org/ampproject/amphtml/jobs/242944862

@zhouyx zhouyx reopened this Jun 14, 2017
@rsimha
Copy link
Contributor Author

rsimha commented Jun 14, 2017

@jridgewell, @camelburrito: These tests are particularly flaky. Can they not just be disabled while they're fixed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants