Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

test(ngOptions): fix flaky test on Firefox 54+ and Safari 9 #16149

Closed
wants to merge 1 commit into from

Conversation

gkalpak
Copy link
Member

@gkalpak gkalpak commented Aug 6, 2017

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Reduce (remove?) test flaky-ness.

What is the current behavior? (You can also link to an open issue here)
A particular ngOptions test is very flaky on Firefox 54+ (possibly due to a (buggy?) optimization).

What is the new behavior (if this is a feature change)?
The test is more reliable on all browsers.

Does this PR introduce a breaking change?
No.

Please check if the PR fulfills these requirements

@gkalpak gkalpak force-pushed the fix-onOptions-flaky-test branch from 4b8f385 to 03b7db6 Compare August 6, 2017 08:16
@gkalpak gkalpak added this to the Backlog milestone Aug 6, 2017
@gkalpak gkalpak changed the title WIP - test(ngOptions): fix flaky test on Firefox 54+ test(ngOptions): fix flaky test on Firefox 54+ Aug 6, 2017
@gkalpak
Copy link
Member Author

gkalpak commented Aug 6, 2017

In case anyone is wondering, this is what caused the issue (afaict):

  • On Firefox and Chrome (didn't check other browsers), the selected property is implemented as a getter/setter property on the HTMLOptionElement prototype (not the <option> element itself).
  • We were defining a getter/setter selected property on the <option> elements and spying on it.
  • It seems that in certain cases (possibly when a specific optimization was triggered), Firefox would ignore our getter/setter spy and use the getter/setter on the prototype (potentially only for get operations).

@Narretz
Copy link
Contributor

Narretz commented Aug 11, 2017

As discussed, I think we should merge this and possibly skip the test altogether on Safari where it also sometimes sporadically fails.

@Narretz
Copy link
Contributor

Narretz commented Aug 11, 2017

Let's merge it as is, and see if there are Safari failures later

@gkalpak gkalpak force-pushed the fix-onOptions-flaky-test branch from 03b7db6 to 9f7d6bc Compare August 11, 2017 21:04
@gkalpak
Copy link
Member Author

gkalpak commented Aug 11, 2017

I disabled the test on Safari 9. Once Travis is green, I'll merge it.
(EDIT: I saw your comment too late 😁 )

@Narretz
Copy link
Contributor

Narretz commented Aug 11, 2017

There still time to revert!

@gkalpak
Copy link
Member Author

gkalpak commented Aug 11, 2017

Too late, I squashed the commits 😛

@gkalpak gkalpak changed the title test(ngOptions): fix flaky test on Firefox 54+ test(ngOptions): fix flaky test on Firefox 54+ and Safari 9 Aug 11, 2017
@gkalpak gkalpak closed this in 7c87628 Aug 12, 2017
gkalpak added a commit that referenced this pull request Aug 12, 2017
@gkalpak gkalpak deleted the fix-onOptions-flaky-test branch February 7, 2018 15:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants