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

Add .bzl files explicitly to toolchain tests to detect .bzl file changes #815

Merged
merged 4 commits into from
Sep 28, 2022

Conversation

fishcakez
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Python toolchain tests did not include the rules_python distribution directly in the tests, and so bazel could cache a toolchain test result even if the toolchain bzl files changed. This could mean that toolchain bugs could be introduced but CI would pass because the tests were cached and did not run.

Issue Number: #806

What is the new behavior?

Add the rules_python distribution to the test files/runfiles so that bazel can detect changes in the bzl files and invalidate the
test cache. Therefore CI should correctly catch test failures.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@google-cla
Copy link

google-cla bot commented Aug 29, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@fishcakez
Copy link
Contributor Author

The CI failure is fixed in #805 and not directly related to this change, though the intent of this PR is to ensure tests/CI are behaving consistently.

@fishcakez
Copy link
Contributor Author

@f0rmiga could you take a look as the author of these tests in #618. Currently folks can break toolchains and won't be caught in CI.

@f0rmiga f0rmiga merged commit 369074e into bazelbuild:main Sep 28, 2022
@fishcakez fishcakez deleted the toolchain-test-deps branch October 26, 2022 00:14
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