taiki-e / cargo-llvm-cov

Cargo subcommand to easily use LLVM source-based code coverage (-C instrument-coverage).
Apache License 2.0
858 stars 57 forks source link

Use default name instead of unwrap when getting workspace root file name #326

Closed MikeDevresse closed 6 months ago

MikeDevresse commented 6 months ago

When trying to execute my tests in a docker container, I encountered the following error which was quite unclear. This PR aims to add more context to this error.

called `Option::unwrap()` on a `None` value
stack backtrace:
   0:     0x7f0ce1e7510f - <unknown>
   1:     0x7f0ce1d6eb7c - <unknown>
   2:     0x7f0ce1e3e4cd - <unknown>
   3:     0x7f0ce1e7696e - <unknown>
   4:     0x7f0ce1e76553 - <unknown>
   5:     0x7f0ce1e77519 - <unknown>
   6:     0x7f0ce1e77018 - <unknown>
   7:     0x7f0ce1e76fa6 - <unknown>
   8:     0x7f0ce1e76f91 - <unknown>
   9:     0x7f0ce1c486d4 - <unknown>
  10:     0x7f0ce1c488a2 - <unknown>
  11:     0x7f0ce1c91911 - <unknown>
  12:     0x7f0ce1ca1f89 - <unknown>
  13:     0x7f0ce1ca9c9a - <unknown>
  14:     0x7f0ce1ca9598 - <unknown>
  15:     0x7f0ce1c63033 - <unknown>
  16:     0x7f0ce1cc7a29 - <unknown>
taiki-e commented 6 months ago

Thanks for the PR. We may want to allow this rather than keep the panic here (by setting the default name).

MikeDevresse commented 6 months ago

The issue is from Utf8Path from camino it says that the function file_name() will return None if we have / as name. I guess you can set a default name (with unwrap_or for example) but I guess you will have to decide what to set, I don't really now what it is used for, I wanted to give extra context for other users that may encounter this.

taiki-e commented 6 months ago

This is only used for the name of the profdata file, so something like unwrap_or("default") would be fine.

taiki-e commented 6 months ago

Published in 0.5.38.