Skip to content

tests: make analysis tests support --incompatible_enable_cc_toolchain_resolution#1281

Merged
chrislovecnm merged 2 commits intobazel-contrib:mainfrom
comius:cc-toolchainisation
Jun 20, 2023
Merged

tests: make analysis tests support --incompatible_enable_cc_toolchain_resolution#1281
chrislovecnm merged 2 commits intobazel-contrib:mainfrom
comius:cc-toolchainisation

Conversation

@comius
Copy link
Copy Markdown
Contributor

@comius comius commented Jun 20, 2023 •
edited by rickeylev
Loading

The analysis tests transition to different platforms to test some platform-specific logic.

When cc toolchain registration is enabled, this also requires that a more complete
toolchain be defined and available.

@comius comius requested a review from rickeylev as a code owner June 20, 2023 17:17
@chrislovecnm
Copy link
Copy Markdown
Contributor

chrislovecnm commented Jun 20, 2023

Buildifier is spotting errors:

tools/build_defs/python/tests/py_test/py_test_tests.bzl:56:13: unsorted-dict-items: Dictionary items are out of their lexicographical order.
--
  | tools/build_defs/python/tests/py_test/py_test_tests.bzl:88:13: unsorted-dict-items: Dictionary items are out of their lexicographical order.

@comius
Copy link
Copy Markdown
Contributor Author

comius commented Jun 20, 2023

Buildifier is spotting errors:

[tools/build_defs/python/tests/py_test/py_test_tests.bzl:56:13](https://github.com/comius/rules_python/blob/c5034d2fc59c38c13ad218ac021ab052e7137733/tools/build_defs/python/tests/py_test/py_test_tests.bzl#L56): [unsorted-dict-items](https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#unsorted-dict-items): Dictionary items are out of their lexicographical order.
--
  | [tools/build_defs/python/tests/py_test/py_test_tests.bzl:88:13](https://github.com/comius/rules_python/blob/c5034d2fc59c38c13ad218ac021ab052e7137733/tools/build_defs/python/tests/py_test/py_test_tests.bzl#L88): [unsorted-dict-items](https://github.com/bazelbuild/buildtools/blob/master/WARNINGS.md#unsorted-dict-items): Dictionary items are out of their lexicographical order.

Fixed.

@chrislovecnm
Copy link
Copy Markdown
Contributor

chrislovecnm commented Jun 20, 2023

If you can push an empty commit, we had a CI flake

@chrislovecnm
Copy link
Copy Markdown
Contributor

chrislovecnm commented Jun 20, 2023

@rickeylev, @f0rmiga or @alexeagle PTAL. This is a little above my knowledge of bazel.

@rickeylev rickeylev changed the title fix: python test to support --incompatible_enable_cc_toolchain_resolution tests: make analysis tests support --incompatible_enable_cc_toolchain_resolution Jun 20, 2023
rickeylev
rickeylev approved these changes Jun 20, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@rickeylev rickeylev left a comment

Choose a reason for hiding this comment

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

for posterity: this just affects some analysis tests of the base rules. They forcibly transition their targets to other platforms, and need some extra fake toolchain stuff defined to work.

@rickeylev rickeylev added this pull request to the merge queue Jun 20, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 20, 2023
@chrislovecnm chrislovecnm added this pull request to the merge queue Jun 20, 2023
Merged via the queue into bazel-contrib:main with commit 0d59fcf Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rickeylev rickeylev rickeylev approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@comius @chrislovecnm @rickeylev