build: share Saucelabs browsers between karma test targets using background Saucelabs daemon and custom karma launcher by gregmagolan · Pull Request #49200 · angular/angular

gregmagolan · 2023-02-24T20:11:23Z

Picks up the work that @devversion started in the components repo.

Plan is to move this to the dev tools in a follow-up.

.circleci/config.yml

devversion

Overall LGTM- but I'm definitely biased a bit 😄

Next steps would be:

Addressing the feedback & landing this
Making sure all parts are tested, like with the legacy job
Replace the legacy job & delete code, in favor of just the Bazel job

.circleci/config.yml

BUILD.bazel

WORKSPACE

browser-providers.conf.d.ts

tools/saucelabs-daemon/background-service/saucelabs-daemon.ts

tools/saucelabs-daemon/launcher/BUILD.bazel

tools/saucelabs-daemon/launcher/launcher.ts

JiaLiPassion

LGTM!
Reviewed-for: fw-zones

dgp1130

Reviewed-for: code-ownership

I'll defer to others on the actual implementation, but I can at least LGTM the PullApprove change.

devversion

LGTM. Couple of more comments, but close. Can you please push feedback as fixup commits?

tools/saucelabs-daemon/background-service/cli.ts

tools/saucelabs-daemon/background-service/ipc.ts

tools/saucelabs-daemon/background-service/saucelabs-daemon.ts

gregmagolan · 2023-05-07T22:42:09Z

Overall LGTM- but I'm definitely biased a bit 😄

Next steps would be:

Addressing the feedback & landing this

Making sure all parts are tested, like with the legacy job

Replace the legacy job & delete code, in favor of just the Bazel job

Ready for another pass @devversion. Changes since last review: e687ce2

devversion

LGTM. final comments. thanks Greg!

scripts/test/run-saucelabs-tests.sh

tools/saucelabs-daemon/background-service/saucelabs-daemon.ts

…ground Saucelabs daemon and custom karma launcher This upgrades the Saucelabs Bazel step on CI to use the more efficient Saucelabs daemon

…s.sh script for local testing more chars to meet the linters requirements

…onal chars to meet commit msg formatting requirements plus more additional chars here

gregmagolan · 2023-05-14T03:35:49Z

Last CI run with saucelabs tests enabled on PRs with passing tests: https://app.circleci.com/pipelines/github/angular/angular/60980/workflows/049e20fc-1957-45c2-b21c-4b0908577ee5/jobs/1335782

Removing the job from this PR now for landing as enabling it will be in a follow-up

…followup additional test to make linter happy

devversion · 2023-05-15T10:57:46Z

scripts/test/run-saucelabs-tests.sh

+# Start the saucelabs-daemon background service in the background. Run directly from the generated
+# bash script instead of using bazel run so we get the PID of the node process. Otherwise killing
+# the child process in kill_background_service doesn't kill the spawn node process.
+cd dist/bin/tools/saucelabs-daemon/background-service/background-service.sh.runfiles/angular


I'm good with the code as is, but it seems a little unexpected to me. the Node process should be a child of the bazel run process, so it should be killed.

It was surprising to me as well. Could use some more investigation in the future.

this is just the convenience script anyway right?

yes, this is not used on CI at all

devversion

LGTM

Reviewed-for: global-dev-infra-approvers

AndrewKushnir · 2023-05-15T16:21:51Z

This PR was merged into the repository by commit d0262cd.

…s.sh script for local testing (#49200) more chars to meet the linters requirements PR Close #49200

…onal chars to meet commit msg formatting requirements (#49200) plus more additional chars here PR Close #49200

…followup (#49200) additional test to make linter happy PR Close #49200

angular-automatic-lock-bot · 2023-06-15T00:09:40Z

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.}

gregmagolan force-pushed the saucelabs_daemon branch 3 times, most recently from c7041de to eb4438a Compare February 24, 2023 22:37

angular-robot bot added the area: build & ci Related the build and CI infrastructure of the project label Feb 24, 2023

ngbot bot added this to the Backlog milestone Feb 24, 2023

gregmagolan force-pushed the saucelabs_daemon branch from eb4438a to 0ffd098 Compare February 24, 2023 22:40

gregmagolan changed the title ~~saucelabs daemon~~ build: share Saucelabs browsers between test targets using background saucelabs daemon Feb 24, 2023

gregmagolan marked this pull request as ready for review February 24, 2023 22:47

pullapprove bot requested a review from josephperrott February 24, 2023 22:47

gregmagolan force-pushed the saucelabs_daemon branch from 0ffd098 to dfeb669 Compare February 24, 2023 22:48

gregmagolan changed the title ~~build: share Saucelabs browsers between test targets using background saucelabs daemon~~ build: share Saucelabs browsers between karma test targets using background Saucelabs daemon and custom karma launcher Feb 24, 2023

gregmagolan force-pushed the saucelabs_daemon branch from dfeb669 to a954864 Compare February 24, 2023 22:50

pullapprove bot requested a review from dylhunn February 24, 2023 22:51

devversion self-requested a review February 24, 2023 22:54

gregmagolan force-pushed the saucelabs_daemon branch 3 times, most recently from d7ce225 to 3c4aca3 Compare February 24, 2023 23:01

gregmagolan commented Feb 24, 2023

View reviewed changes

.circleci/config.yml Outdated Show resolved Hide resolved

gregmagolan force-pushed the saucelabs_daemon branch 6 times, most recently from e90f275 to d61891f Compare February 25, 2023 13:31

pullapprove bot requested a review from JiaLiPassion February 25, 2023 14:22

gregmagolan force-pushed the saucelabs_daemon branch from bb51091 to 8b4672a Compare February 25, 2023 14:37

devversion approved these changes Mar 8, 2023

View reviewed changes

josephperrott removed their request for review March 15, 2023 17:04

pullapprove bot requested a review from dgp1130 March 15, 2023 17:04

JiaLiPassion approved these changes Mar 27, 2023

View reviewed changes

dgp1130 approved these changes Apr 24, 2023

View reviewed changes

devversion approved these changes Apr 25, 2023

View reviewed changes

gregmagolan force-pushed the saucelabs_daemon branch from 5985f04 to 283f782 Compare May 7, 2023 21:10

gregmagolan requested a review from devversion May 7, 2023 22:38

gregmagolan force-pushed the saucelabs_daemon branch 2 times, most recently from 9c601fe to e687ce2 Compare May 7, 2023 22:40

gregmagolan force-pushed the saucelabs_daemon branch from e687ce2 to 0cdcaec Compare May 8, 2023 03:12

devversion approved these changes May 8, 2023

View reviewed changes

build: share Saucelabs browsers between karma test targets using back…

081844d

…ground Saucelabs daemon and custom karma launcher This upgrades the Saucelabs Bazel step on CI to use the more efficient Saucelabs daemon

gregmagolan force-pushed the saucelabs_daemon branch 3 times, most recently from abcf220 to 4954589 Compare May 14, 2023 03:08

gregmagolan added 2 commits May 13, 2023 20:25

build: address review feedback; added scripts/test/run-saucelabs-test…

db1f176

…s.sh script for local testing more chars to meet the linters requirements

build: address review feedback; should be ready to land now... additi…

29891e9

…onal chars to meet commit msg formatting requirements plus more additional chars here

gregmagolan force-pushed the saucelabs_daemon branch from 4954589 to 29891e9 Compare May 14, 2023 03:26

build: don't run saucelabs tests yet on PRs... that will happen in a …

b6591be

…followup additional test to make linter happy

devversion approved these changes May 15, 2023

View reviewed changes

devversion removed request for JiaLiPassion and dylhunn May 15, 2023 10:59

devversion added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels May 15, 2023

AndrewKushnir closed this in 5a9059b May 15, 2023

AndrewKushnir pushed a commit that referenced this pull request May 15, 2023

build: address review feedback; added scripts/test/run-saucelabs-test…

fd35d9c

…s.sh script for local testing (#49200) more chars to meet the linters requirements PR Close #49200

AndrewKushnir pushed a commit that referenced this pull request May 15, 2023

build: address review feedback; should be ready to land now... additi…

51d22fa

…onal chars to meet commit msg formatting requirements (#49200) plus more additional chars here PR Close #49200

AndrewKushnir pushed a commit that referenced this pull request May 15, 2023

build: don't run saucelabs tests yet on PRs... that will happen in a …

d0262cd

…followup (#49200) additional test to make linter happy PR Close #49200

Conversation

gregmagolan commented Feb 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

devversion left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JiaLiPassion left a comment

Choose a reason for hiding this comment

Uh oh!

dgp1130 left a comment

Choose a reason for hiding this comment

Uh oh!

devversion left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gregmagolan commented May 7, 2023

Uh oh!

devversion left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gregmagolan commented May 14, 2023

Uh oh!

devversion May 15, 2023

Choose a reason for hiding this comment

Uh oh!

gregmagolan May 15, 2023

Choose a reason for hiding this comment

Uh oh!

devversion May 15, 2023

Choose a reason for hiding this comment

Uh oh!

gregmagolan May 15, 2023

Choose a reason for hiding this comment

Uh oh!

devversion left a comment

Choose a reason for hiding this comment

Uh oh!

AndrewKushnir commented May 15, 2023

Uh oh!

angular-automatic-lock-bot bot commented Jun 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gregmagolan commented Feb 24, 2023 •

edited

Loading