Skip to content

test(animations): disable buggy test in Chrome 39 - #18483

Merged
vicb merged 1 commit into
angular:masterfrom
marclaval:fixAnimationAndroid51
Aug 9, 2017
Merged

vicb merged 1 commit into
angular:masterfrom
marclaval:fixAnimationAndroid51

Conversation

@marclaval

Copy link
Copy Markdown
Contributor

Quick PR to disable the faulty test reported in #15793, in order to have a better status in CI.

@marclaval marclaval added area: animations legacy animations package only. Otherwise use area: core. action: review The PR is still awaiting reviews from at least one requested reviewer labels Aug 2, 2017
@vicb

vicb commented Aug 7, 2017

Copy link
Copy Markdown
Contributor

Could you add details here on why the test is failing ? (because Android 5 is supported)

@marclaval

Copy link
Copy Markdown
Contributor Author

From my initial investigation, this browser is no longer calculating the height correctly after a web animation, i.e. getComputedStyle is always returning the same value even the element is visually changing size. This explains the test failure.

Android 5 is for sure supported. But in SauceLabs, the emulator is stuck to Mobile Chrome 39, while real Android 5 devices automatically upgrade both Mobile Chrome and the Native WebView.
So I don't feel it is that important to find a workaround for this test failure which occurs only in this outdated browser.

@marclaval
marclaval force-pushed the fixAnimationAndroid51 branch 2 times, most recently from 02993c8 to c64e970 Compare August 8, 2017 13:14
@marclaval

Copy link
Copy Markdown
Contributor Author

@vicb: I've updated the PR to only skip the part which is buggy in this browser.
Debugging in an Android emulator didn't help finding a proper fix.

PTAL

@vicb vicb added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release and removed action: merge The PR is ready for merge by the caretaker action: review The PR is still awaiting reviews from at least one requested reviewer labels Aug 9, 2017
@vicb
vicb merged commit 2f9d8ff into angular:master Aug 9, 2017
vicb pushed a commit that referenced this pull request Aug 9, 2017
asnowwolf pushed a commit to asnowwolf/angular that referenced this pull request Aug 11, 2017
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 26, 2017
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 28, 2017
@marclaval
marclaval deleted the fixAnimationAndroid51 branch December 11, 2017 09:57
@angular-automatic-lock-bot

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: animations legacy animations package only. Otherwise use area: core. cla: yes target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants