knurling-rs / probe-run

Run embedded programs just like native ones
Apache License 2.0
643 stars 75 forks source link

`cargo tests` fails on windows #227

Closed Urhengulas closed 3 years ago

Urhengulas commented 3 years ago

Currently cargo test fails on the windows CI (and are therefore disabled). See https://github.com/knurling-rs/probe-run/runs/2829124012?check_suite_focus=true#step:6:76.

Following tests are reported to fail:

Git lens points me to @japaric, but I will look into this for now.

japaric commented 3 years ago

From a quick glance, these tests contain OS-specific details (the path delimiter). e.g. "/home/user/.cargo/registry/.." is not a valid path on Windows. It's also not the right folder on Windows either.

So either

  1. the tests should become OS-agnostic: the expected strings should not use '/' but rather std::path::MAIN_SEPARATOR, or
  2. the test should be duplicated: one Windows variant ("??\Users\.cargo") and one Unix variant ("/home/user/.cargo") marked with cfg(unix) / cfg(not(unix)) respectively.

Depending on each test we may want to do one (a) or the other (b).

Urhengulas commented 3 years ago

I've started to work on this, but it is a bit hard to test, if you are not working on a windows machine. See https://github.com/knurling-rs/probe-run/tree/fix-winsows-tests

japaric commented 3 years ago

@Urhengulas feel free to assign this issue to me if it's proving to be time consuming. I have a windows machine at hand.

justahero commented 3 years ago

Hello,

I tried to activate some of the tests for Windows to have them run on CI again. In general the use of PathBuf::join helps a lot to make some of the tests pass.

Unfortunately there are some checks inside the different Path::from_std_path implementations that make it pretty difficult to test correctly on a Windows system, for example the function Path::is_absolute is different on Unix & Windows. On Unix systems all path that start with / are considered absolute, while on Windows this requires a drive location, e.g. c:\. For some tests that is not a big issue, because their path use the home directory that can be obtained via the dirs crate to get the OS specific location.

On the other hand I'm not quite sure how to resolve the test dep::rustc::tests::end_to_end, because the path starts with "/rustc/...", where I don't know the equivalent on Windows. Maybe the input path is quite irrelevant and can be easily fixed, otherwise this specific test (& dep::tests::from_std_path_returns_correct_variant) could use a Windows specific setup?

justahero commented 3 years ago

Ok, added the other tests as well in #246, once I realised that the "rustc" part is parsed in a similar fashion as the other Path::from_std_path handle the path parameters. The path does not have to start with "/rustc/...".

Urhengulas commented 3 years ago

Fixed by #246