fix(format): point format_test failure at the multirun fixer#862
fix(format): point format_test failure at the multirun fixer#862ifutivic wants to merge 2 commits into
Conversation
… fixer The FIX_TARGET env var consumed by format.sh was constructed from the test's own per-language target name (e.g. `format_test_Go_with_gofmt`), so a failing `format_test` told users to run the test itself rather than the corresponding `format_multirun` fix command (e.g. `format_Go_with_gofmt`). Add an optional `fix_target` parameter to `format_test` that names the companion `format_multirun` target. When set, the per-language fix target name is derived from it and used as `FIX_TARGET`. The parameter is optional and defaults to the previous behavior, so existing callers are unaffected.
Replaces the hand-rolled _parse_fix_target helper. native.package_relative_label resolves the string relative to the calling BUILD file's package (handling bare names, ':name', and '//pkg:name' uniformly), and .package/.name strip the repo prefix so the constructed FIX_TARGET stays in the user's workspace.
|
| attrs = _format_attr_factory(target_name, lang, toolname, tool_label, "test", disable_git_attribute_checks, custom_args) | ||
| fix_target_label = None | ||
| if fix_label: | ||
| fix_target_label = "//{}:{}_{}_with_{}".format( |
There was a problem hiding this comment.
Why do we have to reformat the label when one exists?
Why do we have a random _with_ in there if the fix_label already has a name?
There was a problem hiding this comment.
fix_label points to a format_multirun target that runs multiple formatters in parallel. Since we want to report which specific formatter failed, we construct the name as format_lang_with_tool.
We have two targets: format and format_test. When format_test fails for Go, for example, we want to tell the user to run format_Go_with_gofmt, matching what the existing format_test failure already reports except with a proper fixer target.
There was a problem hiding this comment.
@jbedard Any updates on this? Happy to adjust or rework anything in the PR. My main goal is just making sure the final error message clearly points users toward the correct fix.
2 participants
A failing
format_testprints:…but that's the test target itself, not a fixer. The
FIX_TARGETenv in_format_attr_factoryis built from the test's own per-language target name (<format_test_name>_<Lang>_with_<tool>) instead of the companionformat_multirun's fix command.Add an optional
fix_targetparameter onformat_testnaming the companionformat_multiruntarget. When set,FIX_TARGETis computed as<fix_target>_<Lang>_with_<tool>in that target's package.Default behavior (parameter omitted) is unchanged.
Usage:
Changes are visible to end-users: yes
RELEASE NOTES: Properly format error message when formatting fails if the corresponding format target is specified