X Tutup
Skip to content

feat(build): initial SauceLabs setup#2347

Closed
marclaval wants to merge 1 commit intoangular:masterfrom
marclaval:saucelabs
Closed

feat(build): initial SauceLabs setup#2347
marclaval wants to merge 1 commit intoangular:masterfrom
marclaval:saucelabs

Conversation

@marclaval
Copy link
Copy Markdown
Contributor

@marclaval marclaval commented Jun 4, 2015

Do not merge, it requires more testing on local machines and on Travis.

This PR introduces the ability to run tests on SauceLabs with various browsers (see issue #2239).

To do so, Sauce Connect needs to be started with valid credentials.
Then run gulp test.unit.js --browsers=option1,option2,..,optionN where options are any mix of browsers and aliases which are defined in the sauce.conf.js file. They are case insensitive, and the SL_ prefix must not be added for browsers.

Some examples of commands:

gulp test.unit.js --browsers=Safari8,ie11  //run on Safari 8 and IE11
gulp test.unit.js --browsers=Safari,IE  //run on Safari 7, Safari 8, IE 9, IE 10 and IE 11
gulp test.unit.js --browsers=IOS,safari8,android5.1  //run on iOS 7, iOS 8, Safari 8 and Android 5.1

For CI, the idea is to run karma with the CI alias defined in the same file.

Review on Reviewable

@marclaval marclaval added feature Label used to distinguish feature request from other issues state: WIP area: build & ci Related the build and CI infrastructure of the project action: review The PR is still awaiting reviews from at least one requested reviewer P3: important labels Jun 4, 2015
@marclaval marclaval force-pushed the saucelabs branch 2 times, most recently from eeb856f to 7267245 Compare June 4, 2015 15:05
vicb
vicb reviewed Jun 4, 2015
View reviewed changes
gulpfile.js Outdated
Copy link
Copy Markdown
Contributor

@vicb vicb Jun 4, 2015

Choose a reason for hiding this comment

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

  • what if you mix sauce & non-sauce
  • throw on unknown browser ?

@marclaval marclaval force-pushed the saucelabs branch 3 times, most recently from 35fad64 to 374cff1 Compare June 4, 2015 16:03
@marclaval
Copy link
Copy Markdown
Contributor Author

marclaval commented Jun 4, 2015

2 issues to be solve (see https://travis-ci.org/angular/angular/jobs/65432819 ):

  1. The 2 XHRImpl unit tests are failing on slow browsers. It sometimes happens to me locally on various browsers (Chrome, Firefox IE11) when running tests while the machine is very busy.
  2. When failures happen in SauceLabs, the build is stalled even though the gulp tasks error. This is reproducible locally.

@marclaval marclaval force-pushed the saucelabs branch 14 times, most recently from b37b272 to 1f7809e Compare June 5, 2015 23:27
@marclaval
Copy link
Copy Markdown
Contributor Author

marclaval commented Jun 5, 2015

All fixed, the PR is ready for review and discussion.

For information, I've tested all the browsers defined in sauce.conf.js.

On the bright side, all the recent ones are green!
This includes Chrome (43, 44, 45), Firefox (37, 39, 40), IE 11, Safari 8, iOS 8, Android 5.1 (Chrome Mobile 39).

Older ones are failing:

  • IE 10: 141 failures
  • IE 9: no run, exception in zone.js (will be fixed in next release)
  • Safari 7: 14 failures
  • iOS 7: 16 failures
  • Android 4.0, 4.1, 4.2, 4.3: no run, exception in zone.js
  • Android 4.4: 14 failures (Chrome Mobile 30)

The questions to discuss:
Should we enable SauceLabs in CI?
If yes, which browsers should we target?

@tbosch tbosch removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Jul 9, 2015
@tbosch tbosch assigned marclaval and unassigned alexeagle Jul 9, 2015
@marclaval marclaval force-pushed the saucelabs branch 2 times, most recently from 7ee48fb to 0837ccb Compare July 21, 2015 16:10
@marclaval
Copy link
Copy Markdown
Contributor Author

marclaval commented Jul 21, 2015

Thanks for the review, some comments integrated.

For the Saucelabs part, I reused the scripts and key management that are used in other angular repositories (angular.js, zone.js).

About the command itself, it is true that gulp test.unit.js --browsers=option1,option2,..,optionN can be confusing as it is a single run, while gulp test.unit.js is not.
To avoid that, a new dedicated task has been added for that: test.unit.js.sauce, which works with the same option.

@marclaval marclaval assigned alexeagle and unassigned marclaval Jul 21, 2015
@marclaval marclaval added action: review The PR is still awaiting reviews from at least one requested reviewer and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Jul 21, 2015
@tbosch
Copy link
Copy Markdown
Contributor

tbosch commented Jul 21, 2015

@alexeagle could you review / set LGTM label?

@IgorMinar
Copy link
Copy Markdown
Contributor

IgorMinar commented Jul 22, 2015

I spoke to Marc about this PR today. Do we want to merge it right now? This
change will slow down our builds and add some degree of instability due to
sauce tunnel. I'd rather add local FF to the mix for now and merge this in
before beta.

On Wed, Jul 22, 2015 at 1:42 AM Tobias Bosch notifications@github.com
wrote:

@alexeagle https://github.com/alexeagle could you review / set LGTM
label?

—
Reply to this email directly or view it on GitHub
#2347 (comment).

@marclaval
Copy link
Copy Markdown
Contributor Author

marclaval commented Jul 23, 2015

It could also be merged without activating Saucelabs in CI until before beta (not needed anyway if we only target Chrome and Firefox).
This way, it would still be possible to target any browser locally with the test.unit.js.sauce task, while not impacting the build time.

@alexeagle
Copy link
Copy Markdown
Contributor

alexeagle commented Jul 23, 2015

That sounds good to me, except for one concern. Will the bits get rusted if
no one is exercising them regularly? I'm imagining "oh, I should try this
with sauce to get some extra browsers" and then discover some needed
maintenance didn't happen.
What about another travis configuration? I don't think we have the option
of returning a result to github PR without waiting for everything.

On Thu, Jul 23, 2015 at 12:58 AM Marc Laval notifications@github.com
wrote:

It could also be merged without activating Saucelabs in CI until before
beta (not needed anyway if we only target Chrome and Firefox).
This way, it would still be possible to target any browser locally with
the test.unit.js.sauce task, while not impacting the build time.

—
Reply to this email directly or view it on GitHub
#2347 (comment).

@marclaval
Copy link
Copy Markdown
Contributor Author

marclaval commented Jul 23, 2015

Well, in our Travis matrix, we could add a special configuration only for testing in SauceLabs . This one would be allowed to fail without making the entire build to fail, see: http://docs.travis-ci.com/user/customizing-the-build/#Rows-that-are-Allowed-to-Fail
It should be anyway faster than the other builds, it could be tested.

This would bring a good insight about the status on non-Chrome browsers, even if it has to be monitored manually.
What do you think?

@alexeagle
Copy link
Copy Markdown
Contributor

alexeagle commented Jul 23, 2015

It will also consume more travis worker slots.
But I think it's worth it to start vetting this setup.

On Thu, Jul 23, 2015 at 3:13 PM Marc Laval notifications@github.com wrote:

Well, in our Travis matrix, we could add a special configuration only for
testing in SauceLabs . This one would be allowed to fail without making the
entire build to fail, see:
http://docs.travis-ci.com/user/customizing-the-build/#Rows-that-are-Allowed-to-Fail
It should be anyway faster than the other builds, it could be tested.

This would bring a good insight about the status on non-Chrome browsers,
even if it has to be monitored manually.
What do you think?

—
Reply to this email directly or view it on GitHub
#2347 (comment).

@marclaval marclaval force-pushed the saucelabs branch 11 times, most recently from eb7436a to 97f4d16 Compare July 24, 2015 10:00
@marclaval
Copy link
Copy Markdown
Contributor Author

marclaval commented Jul 24, 2015

Updated to this approach.
Testing in SauceLabs is now done in a dedicated job, which is kept as simple as possible in order to be fast. It takes around 3 minutes to run unit tests in Chrome and Firefox.
As you can see in Travis, this job can fail without impacting the entire build (3 tests are currently failing in Firefox).

@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Sep 13, 2019

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

@alexeagle alexeagle

Labels

action: review The PR is still awaiting reviews from at least one requested reviewer area: build & ci Related the build and CI infrastructure of the project cla: yes feature Label used to distinguish feature request from other issues

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@marclaval @tbosch @alexeagle @IgorMinar @vicb @googlebot
X Tutup