Skip to content

build: fix small browser test coverage #11970

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 11, 2018
Merged

build: fix small browser test coverage #11970

merged 1 commit into from
Jul 11, 2018

Conversation

devversion
Copy link
Member

@devversion devversion commented Jun 28, 2018 •
edited
Loading

  • No longer just runs the tests against the first browser of a test platform (travis, saucelabs, browserstack)
  • Stability is more valuable than speed (runs sequentially on CI now)
  • Update Saucelabs Connect
  • Remove unused spec reporter
  • Update from Edge 14 to Edge 17
  • Fix pixel deviations in Edge 17

@devversion devversion requested a review from jelbourn as a code owner June 28, 2018 19:19
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jun 28, 2018
@devversion devversion force-pushed the build/fix-no-browser-coverage branch from 90d274f to 097e448 Compare June 29, 2018 12:06
@devversion devversion force-pushed the build/fix-no-browser-coverage branch from bb52ffb to f066104 Compare June 29, 2018 22:02
@devversion devversion requested a review from andrewseguin as a code owner June 29, 2018 22:14
@devversion devversion force-pushed the build/fix-no-browser-coverage branch 2 times, most recently from 899e7dc to ea062ec Compare June 30, 2018 14:20
@devversion
Copy link
Member Author

@jelbourn @josephperrott This is no longer work in progress. I managed to have the tests mostly stable. Browserstack and the Travis mode seem to run perfectly (restarted multiple times).

Saucelabs seems to work fine as well, just sometimes I noticed flakes in Edge. This seems to be caused by something we cannot fix yet (see for example).

I will look into a better stability for Saucelabs in a follow-up. For now the goal is to fix that some browsers don't run at all.

@devversion devversion changed the title WIP build: fix small browser test coverage build: fix small browser test coverage Jun 30, 2018
josephperrott
josephperrott approved these changes Jul 2, 2018
View reviewed changes
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

@josephperrott josephperrott added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Jul 2, 2018
@devversion devversion force-pushed the build/fix-no-browser-coverage branch from ea062ec to f413ee1 Compare July 6, 2018 14:53
@devversion devversion requested a review from crisbeto as a code owner July 7, 2018 08:03
crisbeto
crisbeto reviewed Jul 7, 2018
View reviewed changes
src/lib/select/select.spec.ts Outdated
@@ -2917,11 +2917,15 @@ describe('MatSelect', () => {
const topDifference = Math.floor(optionTop) -
Math.floor(triggerTop - triggerFontSize - triggerExtraLineSpaceAbove);


console.log({optionTop});
console.log({triggerTop}, ' - ', (triggerTop - triggerFontSize - triggerExtraLineSpaceAbove));
Copy link
Member

Choose a reason for hiding this comment

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

Leftover logs?

Copy link
Member Author

@devversion devversion Jul 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

As mentioned on Slack. I'm still debugging right now, so that's why those things are in right now.

@angular angular deleted a comment from crisbeto Jul 7, 2018
@angular angular deleted a comment from crisbeto Jul 7, 2018
@devversion devversion force-pushed the build/fix-no-browser-coverage branch from dbec5d7 to dc3325e Compare July 7, 2018 10:22
@devversion
Copy link
Member Author

Sorry for the commit spam on this PR, but this is just necessary when doing changes that need to be verified with CI builds.

Overall I've updated this PR again because Browserstack no longer supports running mobile devices with our current license. Should be still good to go.

@devversion devversion force-pushed the build/fix-no-browser-coverage branch from dc3325e to 8bde894 Compare July 10, 2018 09:21
jelbourn
jelbourn approved these changes Jul 10, 2018
View reviewed changes
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, just needs rebase

@devversion
build: fix small browser test coverage
b230118 
* No longer just runs the tests against the first browser of a test platform (travis, saucelabs, browserstack)
* Stability is more valuable than speed (runs sequentially on CI now)
* Update Saucelabs Connect
* Remove unused spec reporter
* Update from Edge 14 to Edge 17
* Fix pixel deviations in Edge 17
@devversion devversion force-pushed the build/fix-no-browser-coverage branch from 8bde894 to b230118 Compare July 10, 2018 16:43
@devversion
Copy link
Member Author

Rebased.

@jelbourn jelbourn merged commit fdd7082 into angular:master Jul 11, 2018
@CaerusKaru CaerusKaru mentioned this pull request Jul 19, 2018
Merged
victoriaaa234 pushed a commit that referenced this pull request Jul 25, 2018
@devversion @victoriaaa234
build: fix small browser test coverage (#11970)
004a096 
* No longer just runs the tests against the first browser of a test platform (travis, saucelabs, browserstack)
* Stability is more valuable than speed (runs sequentially on CI now)
* Update Saucelabs Connect
* Remove unused spec reporter
* Update from Edge 14 to Edge 17
* Fix pixel deviations in Edge 17
@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 Sep 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@crisbeto crisbeto crisbeto left review comments

@jelbourn jelbourn jelbourn approved these changes

@josephperrott josephperrott josephperrott approved these changes

@andrewseguin andrewseguin Awaiting requested review from andrewseguin

Assignees

@jelbourn jelbourn

@josephperrott josephperrott

Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@devversion @jelbourn @crisbeto @josephperrott @googlebot