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

test(stdune): Path.drop_prefix doesn't respect path boundaries #8870

Conversation

Alizter
Copy link
Collaborator

@Alizter Alizter commented Oct 6, 2023

This is perhaps an unexpected result of using Path.drop_prefix. Perhaps another function Path.drop_path_prefix is needed instead if this behaviour is intentional.

This is perhaps an unexpected result of using `Path.drop_prefix`.
Perhaps another function `Path.drop_path_prefix` is needed instead if
this behaviour is intentional.

Signed-off-by: Ali Caglayan <[email protected]>
@Alizter Alizter mentioned this pull request Oct 6, 2023
@Alizter
Copy link
Collaborator Author

Alizter commented Oct 6, 2023

@anmonteiro since you implemented this function I am interested to hear what you think about this issue.

@anmonteiro
Copy link
Collaborator

This looks like a bug in drop_prefix instead. I think we want to drop the prefix if it's matched exactly as a path segment.

I think that wouldn't break anything existing usage, but I'm eager to see the results of that test run.

@Alizter Alizter added the bug label Oct 18, 2023
@Alizter Alizter closed this Oct 18, 2023
@Alizter Alizter deleted the ps/branch/test_stdune___path_drop_prefix_doesn_t_respect_path_boundaries branch October 18, 2023 09:07
@Alizter Alizter restored the ps/branch/test_stdune___path_drop_prefix_doesn_t_respect_path_boundaries branch October 18, 2023 09:08
@Alizter Alizter reopened this Oct 18, 2023
@Alizter
Copy link
Collaborator Author

Alizter commented Oct 18, 2023

Reopening as we don't have an issue. But the tests here have been subsumed by #8961.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants