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

fix: Issue with report with Linux file paths #1208

Conversation

jairbubbles
Copy link
Contributor

@SimonCropp
Copy link
Member

can you include a test?

@jairbubbles
Copy link
Contributor Author

jairbubbles commented May 21, 2024

@SimonCropp I didn't really how to test the exact flow so I swapped locally compiled .dll and execute on WSL. It seems that reintroducing the replace removed in your commit 7dc99a9 was fixing the issue.

@jairbubbles
Copy link
Contributor Author

@SimonCropp Yes I'll add a test.

@jairbubbles
Copy link
Contributor Author

@SimonCropp I ended up doing more changes, I fee like it doesn't make much sense to use Path.Combine as we're mixing Linux and Windows paths. As a result, the code is less platform dependent.

@jairbubbles jairbubbles marked this pull request as draft May 21, 2024 15:27
@jairbubbles jairbubbles marked this pull request as ready for review May 21, 2024 16:05
@SimonCropp
Copy link
Member

any updates on some tests in this one?

@jairbubbles
Copy link
Contributor Author

Hi @SimonCropp, in fact it doesn't need a new test case. This case (build on windows and run on linux) was already covered but the test by itself was not properly detecting the issue because of the mocking .

@SimonCropp
Copy link
Member

@jairbubbles thanks for the clarification. will ship this one now

@SimonCropp SimonCropp merged commit c29f356 into VerifyTests:main Jun 2, 2024
3 checks passed
@SimonCropp SimonCropp changed the title fix: Issue with report with Linux file paths when updating from 23.7.1 to 23.7.2 fix: Issue with report with Linux file paths Jun 2, 2024
@SimonCropp SimonCropp added this to the 25.0.1 milestone Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Issue with report with Linux file paths when updating from 23.7.1 to 23.7.2
2 participants