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

run e2e legacy tests under bazel #23074

Merged
merged 4 commits into from Nov 28, 2022
Merged

Conversation

jbedard
Copy link
Contributor

@jbedard jbedard commented May 3, 2022

This runs the legacy-cli/e2e tests via bazel using the bazel built npm packages from packages/**.

After all the pre-factors it's mainly just build config and a few code changes to use the bazel environment vars if present. This can be simplified if the non-bazel versions are dropped in the future. Currently all the CI jobs are prefixed with bazel-.

Build targets / API:

There are 9 bazel targets invoking the e2e tests with different configurations:

  • //tests/legacy-cli:e2e.{npm,yarn,esbuild,saucelabs}: configured like CI today
  • //tests/legacy-cli:e2e.snapshot.{npm,yarn,esbuild,saucelabs}: ^ using snapshots
  • //tests/legacy-cli:e2e: nothing pre-configured, does not run by default or on CI, is designed to be run and configured manually with bazel cli args.

With any of these you can:

  • bazel test to run as a bazel test
  • bazel run to do the equivalent of --debug and remain running on failure
  • --test_filters="..." which essentially does the e2e_runner --glob="..."
  • --test_arg="..." to specify additional e2e_runner args, --glob would work here too

Sharding
Today e2e tests have --shard and --nb-shards cli params which circleci sets based on the parallelism config.
Now those cli args are defaulted based on the bazel environment variables set by nodejs_test(shard_count) and/or the E2E_SHARD_TOTAL + E2E_SHARD environment variables I added. This way both bazel and circleci sharding can be combined into one (bazel shards via processes on one circleci environment, circleci sharding in isolated environments).

Prefactors:

@jbedard jbedard force-pushed the legacy-test branch 30 times, most recently from 1cfd471 to 52f1c3b Compare May 9, 2022 20:58
@jbedard jbedard force-pushed the legacy-test branch 10 times, most recently from 73b5603 to f7210a0 Compare November 8, 2022 08:57
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 though still other comments that appear to be unaddressed.

@jbedard
Copy link
Contributor Author

jbedard commented Nov 14, 2022

I think those were all addressed offline and not in the comments here, just no one had pressed resolve.

Copy link
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

One comment otherwise LGTM. Can you also squash please?

@alan-agius4 alan-agius4 added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Nov 15, 2022
fixup: add ChromeHeadlessNoSandbox browser to to karma builder
@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release 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 Nov 24, 2022
@angular-robot angular-robot bot merged commit 9e57770 into angular:main Nov 28, 2022
@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 Dec 29, 2022
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 target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants