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

Bazel: add a test wrapper around installation scripts #18276

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

redsun82
Copy link
Contributor

This hack is meant to be an optimization when using install for tests, where the install step is skipped if nothing changed. If the installation directory is somehow messed up, bazel run can be used to force install.

This is added as a <name>-installer-as-test target, which we can now use in our internal pytest integration to skip the installation step if nothing changed on the CLI + language packs side.

This hack is meant to be an optimization when using install for tests,
where the install step is skipped if nothing changed. If the
installation directory is somehow messed up, `bazel run` can be used to
force install.

This is added as a `<name>-installer-as-test` target, which we can now
use in our internal pytest integration to skip the installation step if
nothing changed on the CLI + language packs side.
@redsun82 redsun82 requested review from criemen and Copilot December 12, 2024 12:13
@redsun82 redsun82 requested a review from a team as a code owner December 12, 2024 12:13

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

Files not reviewed (1)
  • misc/bazel/pkg.bzl: Language not supported

Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more

Copy link
Collaborator

@criemen criemen left a comment

Choose a reason for hiding this comment

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

I'm not super happy about the UX of this, but this should reduce the pytest snappiness of the inner development loop of integration tests by quite a bit.
We can for sure have this build target exist, and then discuss the UX of actually using the test target on the internal PR for switching pytest over to use it.

],
data = data,
args = args,
size = "small",
Copy link
Collaborator

Choose a reason for hiding this comment

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

the timeout only covers the install part, right? Which under all circumstances ought to finish in 60 seconds, so this is appropriate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed!

@redsun82 redsun82 merged commit 54ba14d into main Dec 16, 2024
22 checks passed
@redsun82 redsun82 deleted the redsun82/bazel-installer-as-test branch December 16, 2024 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants