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

nextest crashes with an (inactionable) error #1090

Closed
ipetkov opened this issue Nov 6, 2023 · 5 comments
Closed

nextest crashes with an (inactionable) error #1090

ipetkov opened this issue Nov 6, 2023 · 5 comments

Comments

@ipetkov
Copy link

ipetkov commented Nov 6, 2023

Hello, thanks for maintaining nextest, it's an awesome test runner!

Recently I tried updating the CI for a large project from using cargo-nextest 0.9.57 to 0.9.61, but I noticed it would consistently fail with the following error:

The application panicked (crashed).
Message:  at least one dependency instance
Location: /private/tmp/nix-build-cargo-nextest-0.9.61.drv-0/cargo-nextest-0.9.61-vendor.tar.gz/guppy/src/graph/build.rs:749

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
                                ⋮ 7 frames hidden ⋮
   8: core::option::expect_failed::hb21837e5cc5baf24
      at <unknown source file>:<unknown line>
   9: guppy::graph::build::<impl guppy::graph::graph_impl::PackageLinkImpl>::new::hb7b48c83156c8168
      at <unknown source file>:<unknown line>
  10: guppy::graph::build::GraphBuildState::process_package::h50ffcdc6f2bbc18a
      at <unknown source file>:<unknown line>
  11: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::try_fold::hf989fe44753715c4
      at <unknown source file>:<unknown line>
  12: core::iter::adapters::try_process::hc9dc0afb22ce8b8e
      at <unknown source file>:<unknown line>
  13: guppy::graph::graph_impl::PackageGraph::from_metadata::h95196efeeeafcbb9
      at <unknown source file>:<unknown line>
  14: cargo_nextest::dispatch::BaseApp::new::h85a535096f0eff96
      at <unknown source file>:<unknown line>
  15: cargo_nextest::dispatch::AppOpts::exec::h2857cb9a8a41c53b
      at <unknown source file>:<unknown line>
  16: cargo_nextest::dispatch::CargoNextestApp::exec::h0d0c6712119da5fb
      at <unknown source file>:<unknown line>
  17: cargo_nextest::main::h8e61fa02158f5282
      at <unknown source file>:<unknown line>
  18: std::sys_common::backtrace::__rust_begin_short_backtrace::h69eecc6337efd2d7
      at <unknown source file>:<unknown line>
  19: std::rt::lang_start::{{closure}}::h3d89c02499118a25
      at <unknown source file>:<unknown line>
  20: std::rt::lang_start_internal::h6f6bdb9ebc0beefc
      at <unknown source file>:<unknown line>
  21: _main<unknown>
      at <unknown source file>:<unknown line>

We've got a pretty large code base which I'm not exactly sure how to minimize into a representative test case, but if you give me a nudge in what might be the problem (something to do with dependencies perhaps?) I can try to make a minimal reproduction that's safe to share.

It looks like this might have been introduced in 0.9.59 (since 0.9.58 seems to work fine), and all later versions have the same issue.

(I do see that the error itself is coming from guppy so I apologize if I've opened an issue in the wrong place, but figured there might be more interesting context here)

@sunshowers
Copy link
Member

Thanks, that's really strange. It's coming from here: https://github.com/guppy-rs/guppy/blob/1b84e0514a5a8ecf060adc8c081f62e7875427f5/guppy/src/graph/build.rs#L721

This can only happen if we tried to create a PackageLink when there isn't actually a dependency between two packages. That shouldn't normally happen.

There is a change in nextest 0.9.59 to always pull in dependency information into guppy, which would explain why this issue occurred for you as a side effect.

Would you be comfortable sharing the output of cargo metadata? Either attaching it here or emailing it as an attachment to me at rain @ sunshowers.io.

@ipetkov
Copy link
Author

ipetkov commented Nov 7, 2023

I actually managed to get a minimal reproduction (wonders what a good night's sleep and some println debugging will do)!

Basically found an equivalent of this in our code base for reasons (which aren't important), but it is a valid cargo representation. Here's how to create a repro:

cargo new nextest-repro --lib
cd nextest-repro

cat >>Cargo.toml <<EOF
[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
serde_json = "1"
[target.'cfg(target_arch = "wasm32")'.dependencies]
serde_json = { package = "serde-json-wasm", version = "0.4" }
EOF

cargo nextest run

Avoiding renaming like this is a viable workaround for anyone else who runs into this!

@sunshowers
Copy link
Member

Thanks! That's a really interesting case.

@sunshowers
Copy link
Member

guppy-rs/guppy#148 is my attempt to start fixing this, but this is more complex than I thought it would be so I'm going to shelve this for now. There are some very subtle issues around this though.

sunshowers added a commit to sunshowers/guppy that referenced this issue Nov 11, 2023
We used to use a heuristic matching algorithm which would check a renamed map
first, then check an original map. We know that that was failing to work in a
few cases, and we encountered one such case in the wild in
nextest-rs/nextest#1090.

Fix that through a few different things:

1. Match up the resolved name correctly, using the same algorithm Cargo uses.
2. Use the `dep_kinds` list introduced in Rust 1.41 (see
   rust-lang/cargo#5583).
sunshowers added a commit to sunshowers/guppy that referenced this issue Nov 11, 2023
We used to use a heuristic matching algorithm which would check a renamed map
first, then check an original map. We know that that was failing to work in a
few cases, and we encountered one such case in the wild in
nextest-rs/nextest#1090.

Fix that through a few different things:

1. Match up the resolved name correctly, using the same algorithm Cargo uses.
2. Use the `dep_kinds` list introduced in Rust 1.41 (see
   rust-lang/cargo#5583).
sunshowers added a commit to guppy-rs/guppy that referenced this issue Nov 11, 2023
We used to use a heuristic matching algorithm which would check a renamed map
first, then check an original map. We know that that was failing to work in a
few cases, and we encountered one such case in the wild in
nextest-rs/nextest#1090.

Fix that through a few different things:

1. Match up the resolved name correctly, using the same algorithm Cargo uses.
2. Use the `dep_kinds` list introduced in Rust 1.41 (see
   rust-lang/cargo#5583).
sunshowers added a commit that referenced this issue Nov 14, 2023
@sunshowers
Copy link
Member

A fix for this is part of cargo-nextest 0.9.62, which should be out within the next 20 minutes.

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

No branches or pull requests

2 participants