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

Fix race condition in embedding tests #8507

Merged

Conversation

joheriks
Copy link
Contributor

@joheriks joheriks commented Jun 5, 2020

Fixes #8329. With Chrome dropping native HTML imports and the compatibility mode test for embedded webcomponents relying on polyfill, a race condition had appeared where the shadowroot was not yet attached during test run. Now waits for at least one shadowroot before commencing the test.

Update: should squash when merging.

// attached #8329
this.waitUntil((driver) -> Boolean.TRUE
.equals(this.getCommandExecutor().executeScript(
"return Array.from(document.getElementsByTagName('*')).some(x => x.shadowRoot !== null)")));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

x => x.shadowRoot

That didn't work in IE.
Though we don't run our tests in IE but still : is it possible to write this somehow cross-browser ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ES5 compat JS.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, sorry, I meant x.shadowRoot .
I don't know actually what's the output of the compiled production client side code.

But for the previous releases production code produced a code which worked in browsers which don't support web components at all .
In those browsers shadowRoot just don't exist at all.

E.g. in IE11 (again: I don't know which browsers we support nowadays) TB has a specific code which checks the element directly in the parent and then tries to use shadowRoot.

And IE11 of course doesn't support =>.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened the test view in IE8, and the embedded web components just appear as empty tags in the DOM. I don't know if the test is designed to work in IE at all?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IE 8 ?

IE 8 is not supported for sure.

IE 11 : I don't know.

But yeah, may be it has no sense to care about this at the moment.

@denis-anisimov denis-anisimov merged commit bf1a3aa into 2.2 Jun 5, 2020
@denis-anisimov denis-anisimov deleted the joheriks/8329-fix-race-condition-in-embedding-tests branch June 5, 2020 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants