Skip to content

Fix tests on Windows #54

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

Merged
merged 6 commits into from
Apr 24, 2025
Merged

Conversation

FlippingBinary
Copy link
Contributor

This pull request fixes failing tests on Windows, possibly resolving #10. It is not intended to represent a comprehensive review of the code or functionality on Windows, but it does fix the bug that was causing certain tests to fail. The problem was fixed by using folder.uri.to_file_path().unwrap() instead of PathBuf::from(folder.uri.path()) in the diagnostics method. The .to_file_path() method handles Windows paths correctly, but the .unwrap() introduces the possibility of a panic if diagnostics is ever called with a non-file URL. It should probably use a sensible default instead, or at least pass the error in its return value.

A few other changes in this pull request include the removal of an unused import, swapping cargo --version into the background test so that test doesn't fail in PowerShell environments, and enabling a test that was previously disabled on Windows.

…un_in_background`

PowerShell uses a built-in for `echo`, which doesn't play well with the test. I replaced it with a call to `cargo` because that should be available in the path of any machine that runs `cargo test`.
@crisidev
Copy link
Owner

This is awesome, thanks a million for the contribution.

@crisidev
Copy link
Owner

Don't worry about the code-coverage check failing, it's because it's using a token scoped to my user. I should fix it sooner or later.

crisidev and others added 2 commits April 24, 2025 13:30
@crisidev crisidev merged commit f02ec0d into crisidev:main Apr 24, 2025
3 of 4 checks passed
@FlippingBinary FlippingBinary deleted the fix/tests-on-windows branch April 24, 2025 13:18
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