Skip to content

Conversation

@Pyr0de
Copy link
Contributor

@Pyr0de Pyr0de commented Jun 4, 2025

Added tests from https://github.com/rust-lang/cargo/blob/f1bf94d3b2a779d14e2ad6dff43f8d8017583b0d/tests/testsuite/fix.rs

Commented out tests for Edition
Tests pass for fix

Tests pass for fix_n_times
sorry about commit 553a5e4, forgot to commit my changes before making all the changes

@Pyr0de Pyr0de force-pushed the tests branch 2 times, most recently from d77131a to c03ddf3 Compare June 6, 2025 06:18
@coveralls
Copy link

coveralls commented Jun 6, 2025

Pull Request Test Coverage Report for Build 15496345075

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage remained the same at 0.0%

Files with Coverage Reduction New Missed Lines %
src/errors.rs 1 0.0%
Totals Coverage Status
Change from base Build 15423780252: 0.0%
Covered Lines: 0
Relevant Lines: 20

💛 - Coveralls

@Pyr0de Pyr0de force-pushed the tests branch 2 times, most recently from 1bd2c91 to 6ff2c20 Compare June 6, 2025 07:13
@Pyr0de Pyr0de changed the title test: Added tests from cargo, test: Added tests from cargo Jun 6, 2025
@Pyr0de Pyr0de marked this pull request as ready for review June 6, 2025 16:17
@epage
Copy link
Contributor

epage commented Jun 6, 2025

Could you clean up the commit history?

  • deleting delete_me.rs would be its own commit
  • The rest would likely be its own commit as well

@epage
Copy link
Contributor

epage commented Jun 6, 2025

Looks good, thanks for doing this!

@Pyr0de
Copy link
Contributor Author

Pyr0de commented Jun 6, 2025

Should I make each file as a single commit?

@epage
Copy link
Contributor

epage commented Jun 6, 2025

Either way.

@epage epage merged commit 6638a9c into crate-ci:main Jun 6, 2025
15 checks passed
@epage
Copy link
Contributor

epage commented Jun 6, 2025

Thanks!

@Pyr0de Pyr0de deleted the tests branch June 7, 2025 04:36
github-merge-queue bot pushed a commit to rust-lang/cargo that referenced this pull request Jun 26, 2025
…r Cargo (#15692)

### What does this PR try to resolve?

This PR reworks `cargo-test-support` and `testsuite` to use Snapbox's
[`cargo_bin!()`](https://docs.rs/snapbox/latest/snapbox/cmd/macro.cargo_bin.html)
instead of
[`cargo_bin()`](https://docs.rs/snapbox/latest/snapbox/cmd/fn.cargo_bin.html)
which makes assumptions about the structure of Cargo's build directory.
`cargo_bin!()` uses `CARGO_BIN_EXE_*` for locating the `cargo` binary
which should be more resilient to directory/layout changes.

Linking a relevant Zulip discussion
[here](https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/cargo_bin_exe.20and.20tests/with/513638220)[#t-cargo
> cargo_bin_exe and
tests](https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/cargo_bin_exe.20and.20tests/with/513638220)

As shown in that link, we could make these variables available at
runtime and not need to do this. However, `cargo-test-support`, as an
API, is a bit weird in that it is baking in support for one specific
binary. This can be confusing for callers and makes it more annoying for
callers provide their own `fn cargo`, e.g. see crate-ci/cargo-fixit#7

### Implementation Notes

`cargo_bin!()` only works when being called from the `testsuite` as it's
only set when executing integration tests and `cargo-test-support` is a
regular crate.
To make this change, I introduced an extension trait `CargoProjectExt`
in `testsuite` for running `.cargo()` and implemented it on `Project`.

In `cargo-test-support` other functionality relies on `.cargo()` so
these also needed to be moved to `testsuite`
*
[`src/tools.rs`](https://github.com/rust-lang/cargo/blob/master/crates/cargo-test-support/src/tools.rs)
* Parts
[`src/cross_compile`](https://github.com/rust-lang/cargo/blob/master/crates/cargo-test-support/src/cross_compile.rs)
* I had to split this up unfortunately, as `disabled()` requires running
Cargo to check if we should disable cross compile tests.
* Other fns in `cross_compile` are used in `cargo-test-support` so
moving everything to `testsuite` would have ended up requiring moving
many things to test suite.

### How to test and review this PR?

I'd definitely recommend reviewing commit by commit.
There are a lot of diffs due to the nature of reorganizing things.
I did my best to split things things into smaller PRs but they still
contain a lot of `use` statement diffs.

r? @epage
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.

3 participants