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

Ensure tests are built with same dependencies as parent crate #53

Closed
dtolnay opened this issue Mar 9, 2020 · 6 comments · Fixed by #102
Closed

Ensure tests are built with same dependencies as parent crate #53

dtolnay opened this issue Mar 9, 2020 · 6 comments · Fixed by #102

Comments

@dtolnay
Copy link
Owner

dtolnay commented Mar 9, 2020

The crate that gets compiled by trybuild is given the same dependencies in its Cargo.toml as the outer crate has. But it may have an old Cargo.lock left over from a previous cargo test run that causes it to use a different set of patch versions of those dependencies compared to what is used currently by the outer crate whose tests we are running. To avoid this, trybuild should somehow write a Cargo.lock (or failing that, use a sequence of cargo update --precise commands) to keep the resolved set of dependencies in sync.

@dtolnay dtolnay added the help wanted Extra attention is needed label Mar 9, 2020
@dtolnay dtolnay changed the title Write a Cargo.lock to ensure tests are built with same dependencies as parent crate Ensure tests are built with same dependencies as parent crate Mar 9, 2020
@TriplEight
Copy link

TriplEight commented Mar 9, 2020

Steps to repro:

# download the cache from https://drive.google.com/file/d/1J3ZqSTP_yRugPcx6aIohiHdhDxZ7A3Qp/view?usp=sharing
# unzip the cache to ~/cache/targets/
docker run -it -v ~/cache/:/ci-cache parity/ink-ci-linux:demo

# inside of the container:
export SCCACHE_DIR=/ci-cache/sccache
export CARGO_HOME=/ci-cache/cargo
export CARGO_TARGET_DIR=/ci-cache/targets/337/test/

git clone https://github.com/paritytech/ink .
git checkout 79f8571172892426161d70adc7d514767dcc3c9b

cargo test --verbose --all-features --release --manifest-path lang/macro/Cargo.toml --test compile_tests

@TriplEight
Copy link

locally I'm getting

running 1 test
error: failed to download `aho-corasick v0.7.9`

Caused by:
  can't make HTTP request in the offline mode
test compile_tests ... FAILED

@ascjones
Copy link
Contributor

A naive solution would be to remove the cached Cargo.lock from the location of the crate generated by trybuild. e.g. master...ascjones:rm-cargo-lock.

It will then write a new Cargo.lock which will resolve the best set of dependencies available locally (because of --offline) which should be the same as those downloaded for the outer crate.

I've tested this for the repro above and it works, and rerunning the tests with the package resolution is fast.

I'm not sure though that this will guarantee the exact same set of dependencies, or what other unintended consequences there may be. But putting this suggestion forward as the "simplest thing that will possibly work".

Another idea I'm currently investigating is when copying the dependencies to the crate generated by trybuild, read the precise versions from the outer crate's Cargo.lock and write them to the generated Cargo.toml. In this way we would guarantee the same set of dependencies.

@dtolnay let me know what you think of those ideas and I can prepare a PR, or go back to the drawing board.

@ascjones
Copy link
Contributor

ascjones commented Mar 10, 2020

As an alternative to deleting the Cargo.lock, running cargo generate-lockfile --offline would force refresh the Cargo.lock.

Tried it and it works: master...ascjones:generate-lockfile

Edit: PR for this 👇

@dtolnay
Copy link
Owner Author

dtolnay commented Mar 10, 2020

I published the partial workaround in #54 in 1.0.24. Thanks @ascjones!

@cecton
Copy link

cecton commented Oct 16, 2020

I'm still getting this issue on version 1.0.35

cecton added a commit to paritytech/substrate that referenced this issue Oct 16, 2020
ghost pushed a commit to paritytech/substrate that referenced this issue Oct 21, 2020
* Initial commit

Forked at: d67fc4c
Parent branch: origin/master

* WIP

Forked at: d67fc4c
Parent branch: origin/master

* WIP

Forked at: d67fc4c
Parent branch: origin/master

* WIP

Forked at: d67fc4c
Parent branch: origin/master

* WIP

Forked at: d67fc4c
Parent branch: origin/master

* WIP

Forked at: d67fc4c
Parent branch: origin/master

* CLEANUP

Forked at: d67fc4c
Parent branch: origin/master

* Add notes to original source code

* CLEANUP

Forked at: d67fc4c
Parent branch: origin/master

* CLEANUP

Forked at: d67fc4c
Parent branch: origin/master

* WIP

Forked at: d67fc4c
Parent branch: origin/master

* WIP

Forked at: d67fc4c
Parent branch: origin/master

* WIP

Forked at: d67fc4c
Parent branch: origin/master

* CLEANUP

Forked at: d67fc4c
Parent branch: origin/master

* WIP

Forked at: d67fc4c
Parent branch: origin/master

* Some doc

* Test with trybuild

* Revert "Test with trybuild" (issue with trybuild atm)

This reverts commit 9055ec2.

dtolnay/trybuild#53

* Apply suggestions

* Rename derive to proc-macro

* Remove "prefix" feature from informant

* Blocking task should use SpawnHandle::spawn_blocking

* Improve doc as suggested

* Fixes

Forked at: d67fc4c
Parent branch: origin/master

* Apply suggestion

* Update client/cli/proc-macro/src/lib.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* More suggestions

* CLEANUP

Forked at: d67fc4c
Parent branch: origin/master

* Improve error message

* CLEANUP

Forked at: d67fc4c
Parent branch: origin/master

* Fix async issue

* CLEANUP

Forked at: d67fc4c
Parent branch: origin/master

* CLEANUP

Forked at: d67fc4c
Parent branch: origin/master

* Add test

* fix doc test

* Update client/cli/src/logging.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Update client/basic-authorship/src/basic_authorship.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Update client/basic-authorship/src/basic_authorship.rs

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

* Apply suggestions

* Suggestions

* Clarify doc

* WIP

Forked at: d67fc4c
Parent branch: origin/master

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
@dtolnay dtolnay removed the help wanted Extra attention is needed label Oct 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants