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

Allow analysis tests to support for_dependency_resolution on the target_under_test #553

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Bocete
Copy link
Contributor

@Bocete Bocete commented Nov 15, 2024

This attribute is necessary to analysis test dormant targets. It's hard to assert anything useful without materializing what's behind the target_under_test, and that requires setting for_dependency_resolution on the target_under_test.

See https://bazel.build/rules/lib/toplevel/attr#label.for_dependency_resolution for more info on this attribute. It's a new bazel feature.

@Bocete
Copy link
Contributor Author

Bocete commented Nov 15, 2024

I suspect this is blocked on a blaze release? Dormant deps are still experimental, and the CI tests are failing because of syntax that's made available via experimental flags. @lberki to comment

@lberki
Copy link

lberki commented Nov 18, 2024

I don't think it's technically blocked on a Blaze release, only its use internally is for reasons complicated and irrelevant to Bazel. So as long as the code is fine, feel free to merge it.

@Bocete
Copy link
Contributor Author

Bocete commented Nov 18, 2024

Hmm the build checks, here on github, are failing because:

(23:17:46) ERROR: Traceback (most recent call last):
	File "/workdir/tests/unittest_tests.bzl", line 284, column 28, in <toplevel>
		"dormant_dep": attr.dormant_label(),
Error: 'attr' value has no field or method 'dormant_label'

That's telling me something needs to be enabled blaze-side, no? Or maybe the CI system here needs to be tweaked to add a flag? Please tell me what to do here, thanks!

@lberki
Copy link

lberki commented Nov 19, 2024

I'm not familiar with the unit testing framework so take the reviewers' advice over mine, but how about adding something like this:

if hasattr(attr, "dormant_label"):
  inspect_dormant_target_fake_rule = rule(attrs = {"dormant_dep": attr.dormant_label()})
else:
  inspect_dormant_target_fake_rule = rule(implementation = _always_fail)

that way, Skylib remains compatible with older Bazel versions while still allowing you to add these symbols.

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