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

Flaky tests after resource changes #61

Closed
cramforce opened this issue Sep 11, 2015 · 13 comments
Closed

Flaky tests after resource changes #61

cramforce opened this issue Sep 11, 2015 · 13 comments
Assignees
Milestone

Comments

@cramforce
Copy link
Member

To repro:

Scroll to YT videos.
Click reload.
The document doesn't load uniformly.

Lets find out why. This should not happen.

CC @dvoytenko

@dvoytenko
Copy link
Contributor

Hmm. Tried few times. I don't see it jumping around. Are you trying on your phone? Or emulator?

@erwinmombay
Copy link
Member

same here, BUT i do see amp-twitter jumping around. ill ping you guys video through mail.

edit: @dvoytenko explained that this was a known problem

@dvoytenko
Copy link
Contributor

So, twitter jumping is due to some scale/resize sequence. We tried to address it before, but I guess we didn't do so fully. It jumping around however would not cause any relayouts and thus I think what Malte is describing is a different thing.

One thing does cause some concern is how we apply "media" attribute. This could possibly cause a risk condition where we apply "media" attribute a little too late which would cause a visible effect. Not sure if this is the reason here.

@cramforce
Copy link
Member Author

We should definitely make sure to apply media as early as possible.
On Sep 10, 2015 10:31 PM, "Dima Voytenko" [email protected] wrote:

So, twitter jumping is due to some scale/resize sequence. We tried to
address it before, but I guess we didn't do so fully. It jumping around
however would not cause any relayouts and thus I think what Malte is
describing is a different thing.

One thing does cause some concern is how we apply "media" attribute. This
could possibly cause a risk condition where we apply "media" attribute a
little too late which would cause a visible effect. Not sure if this is the
reason here.


Reply to this email directly or view it on GitHub
#61 (comment).

@cramforce
Copy link
Member Author

The amp-img test for media queries is consistently failing for me. Lets try to fix that ASAP.

Also the test runner that @erwinmombay fixed is broken again. Tests basically only run once and then require a restart.

@erwinmombay
Copy link
Member

@cramforce noticed that yesterday. not sure what happened, I'll take a look again.

@erwinmombay
Copy link
Member

@cramforce will also investigate and fix failing test

@cramforce cramforce changed the title everything.html jumps around Flaky tests after resource changes Sep 11, 2015
@cramforce
Copy link
Member Author

Also the iframe tests are flaky

@cramforce
Copy link
Member Author

@erwinmombay This is most likely due to @dvoytenko's change. I think he already had an idea how to fix the media queries.

@cramforce cramforce modified the milestone: Preview Sep 11, 2015
@erwinmombay
Copy link
Member

#104 turned off the flaky tests for now.

as @cramforce has linked we'll need to fix this before preview.

@cramforce
Copy link
Member Author

The tests still run locally, so we don't forget!

Also I haven't turned them all of I think. There are some tests that are super flaky on Travis. Those no longer run there, but some tests are flaky everywhere (maybe 10%) and those I left on :)

@erwinmombay
Copy link
Member

@cramforce this should be good to close? or do we still have some really bad flaky tests? (think you've fixed it for the most part)

@erwinmombay
Copy link
Member

@dvoytenko has been running into some consistent ones that only he can repro (carousel ones specifically), will take a look when i get a chance and might reopen this.

kristoferbaxter pushed a commit that referenced this issue Sep 9, 2021
* Add JW Player to documentation that applies

* Revert "Add JW Player to documentation that applies"

This reverts commit 5aeba5a.

* Extend AMP Params (#62)

* Add setup method

* create separate postMessage method and add setup test

* adding support for data-params and querystring

* updating tests for data-params

* re-adding unit test i accidentally removed

* Pass custom config added through amp to player

* Use correct import format

* updated attribute names

* adding code suggestions

* Add test coverage and fix formatting

* Move test config to above assertation

* Update media id and player id for vast ad config

* Add cust-params data attribute for ads

* testing

* Integrate with amp-consent and add custom macros

* Lint fixes

Co-authored-by: Daniel Lee <[email protected]>

* Remove bad import

* Clean up consentgdpr

* Add comment about appending querystring

* Clean up from rebase

* Remove trailing whitespaces

* Remove controls attribute

* 📖  [JW8-11873] Update JW Player AMP Docs (#61)


    Update our amp-jwplayer docs with the new attributes
    Add amp-jwplayer in the docs where it applies

* cleanup rebase

* lint fixes

Co-authored-by: Zack Haigh <[email protected]>
Co-authored-by: Daniel Lee <[email protected]>
rbeckthomas pushed a commit to rbeckthomas/amphtml that referenced this issue Sep 14, 2021
* Add JW Player to documentation that applies

* Revert "Add JW Player to documentation that applies"

This reverts commit 5aeba5a.

* Extend AMP Params (ampproject#62)

* Add setup method

* create separate postMessage method and add setup test

* adding support for data-params and querystring

* updating tests for data-params

* re-adding unit test i accidentally removed

* Pass custom config added through amp to player

* Use correct import format

* updated attribute names

* adding code suggestions

* Add test coverage and fix formatting

* Move test config to above assertation

* Update media id and player id for vast ad config

* Add cust-params data attribute for ads

* testing

* Integrate with amp-consent and add custom macros

* Lint fixes

Co-authored-by: Daniel Lee <[email protected]>

* Remove bad import

* Clean up consentgdpr

* Add comment about appending querystring

* Clean up from rebase

* Remove trailing whitespaces

* Remove controls attribute

* 📖  [JW8-11873] Update JW Player AMP Docs (ampproject#61)


    Update our amp-jwplayer docs with the new attributes
    Add amp-jwplayer in the docs where it applies

* cleanup rebase

* lint fixes

Co-authored-by: Zack Haigh <[email protected]>
Co-authored-by: Daniel Lee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants