Skip to content

build: run edge tests on browserstack #20662

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

Merged
merged 1 commit into from
Sep 29, 2020

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Sep 27, 2020

Moves the Edge tests to Browserstack to avoid the recent issues with Saucelabs.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Sep 27, 2020
@crisbeto crisbeto force-pushed the karma-launchers-update branch 2 times, most recently from 940398a to 37ba1c0 Compare September 27, 2020 08:48
@crisbeto crisbeto closed this Sep 27, 2020
@crisbeto crisbeto reopened this Sep 27, 2020
@crisbeto crisbeto force-pushed the karma-launchers-update branch 9 times, most recently from 55aa082 to 0f60f4c Compare September 27, 2020 13:29
Moves the Edge tests to Browserstack to avoid the recent issues with Saucelabs.
@crisbeto crisbeto force-pushed the karma-launchers-update branch from 0f60f4c to 78b6d8d Compare September 27, 2020 14:27
@crisbeto crisbeto changed the title build: update karma dependencies build: run edge tests on browserstack Sep 27, 2020
@crisbeto crisbeto marked this pull request as ready for review September 27, 2020 14:46
@crisbeto crisbeto requested review from devversion, mmalerba and a team as code owners September 27, 2020 14:46
@crisbeto crisbeto added merge safe P2 The issue is important to a large percentage of users, with a workaround target: patch This PR is targeted for the next patch release labels Sep 27, 2020
@@ -354,23 +354,23 @@ export function crossEnvironmentSpecs(
});

it('should be able to hover', async () => {
const host = await harness.host();
let classAttr = await host.getAttribute('class');
const box = await harness.hoverTest();
Copy link
Member Author

@crisbeto crisbeto Sep 27, 2020

Choose a reason for hiding this comment

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

I decided to change these tests to hover an element inside the host, instead of the host itself, because the first assertion, that ensures that the element doesn't start off as hovered, was breaking on Browserstack. What I think was going on is that it would put the cursor inside the viewport when the window is opened, throwing off our assertion since the host takes up most of the screen and the cursor would land on it.

Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added the action: merge The PR is ready for merge by the caretaker label Sep 28, 2020
@mmalerba mmalerba merged commit 8b6aab1 into angular:master Sep 29, 2020
mmalerba pushed a commit that referenced this pull request Sep 29, 2020
Moves the Edge tests to Browserstack to avoid the recent issues with Saucelabs.

(cherry picked from commit 8b6aab1)
@angular-automatic-lock-bot
Copy link

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 Oct 30, 2020
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 cla: yes PR author has agreed to Google's Contributor License Agreement P2 The issue is important to a large percentage of users, with a workaround target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants