Skip to content

ArgSplitter operates relative to the buildroot.#10540

Merged
Eric-Arellano merged 2 commits intopantsbuild:masterfrom
stuhood:stuhood/arg-splitter-buildroot
Aug 3, 2020
Merged

ArgSplitter operates relative to the buildroot.#10540
Eric-Arellano merged 2 commits intopantsbuild:masterfrom
stuhood:stuhood/arg-splitter-buildroot

Conversation

@stuhood
Copy link
Copy Markdown
Member

@stuhood stuhood commented Aug 3, 2020

Problem

#10535 normalizes specs to not include the trailing name when it will default. In order to fix tests, many instances of src/example/test:test in output (the form is still supported and normalized in input) will be converted to src/example/test. To avoid needing to differentiate input from output though, and allow for a cleaner find-replace, I'm going to change all instances.

While doing so, I noticed that the ArgSplitter operates relative to the current directory rather than the buildroot, which meant that GoalRuleTestBase was not recognizing file args or defaulted directory specs correctly.

Solution

Adjust ArgSplitter to operate relative to the buildroot.

[ci skip-rust]
[ci skip-build-wheels]

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
@stuhood stuhood requested review from Eric-Arellano and benjyw August 3, 2020 19:06
Copy link
Copy Markdown
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Thanks!

Comment on lines -72 to -76
def assert_spec(arg: str) -> None:
assert ArgSplitter.likely_a_spec(arg) is True

def assert_not_spec(arg: str) -> None:
assert ArgSplitter.likely_a_spec(arg) is False
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for removing these. I agree it's more readable without these.

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 0.0% when pulling 79cf03c on stuhood:stuhood/arg-splitter-buildroot into 5d79f42 on pantsbuild:master.

@Eric-Arellano Eric-Arellano merged commit b2c1cba into pantsbuild:master Aug 3, 2020
@stuhood stuhood deleted the stuhood/arg-splitter-buildroot branch August 3, 2020 20:03
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.

3 participants