Use depinfo to discover UI test dependencies#11045
Conversation
|
Some numbers, $ hyperfine --warmup 2 \
--runs 5 \
--parameter-list commit master,extern-flags \
--parameter-list package clippy_lints,clippy_utils \
--setup "git checkout {commit}" \
--prepare "touch {package}/src/lib.rs" \
--command-name "{commit}, rebuild {package}" \
"TESTNAME=___ cargo uitest"
Benchmark 1: master, rebuild clippy_lints
Time (mean ± σ): 21.468 s ± 0.152 s [User: 16.322 s, System: 3.989 s]
Range (min … max): 21.268 s … 21.645 s 5 runs
Benchmark 2: extern-flags, rebuild clippy_lints
Time (mean ± σ): 11.418 s ± 0.084 s [User: 8.734 s, System: 2.397 s]
Range (min … max): 11.309 s … 11.536 s 5 runs
Benchmark 3: master, rebuild clippy_utils
Time (mean ± σ): 24.386 s ± 0.173 s [User: 19.307 s, System: 5.706 s]
Range (min … max): 24.127 s … 24.561 s 5 runs
Benchmark 4: extern-flags, rebuild clippy_utils
Time (mean ± σ): 12.984 s ± 0.160 s [User: 10.173 s, System: 3.316 s]
Range (min … max): 12.791 s … 13.233 s 5 runsFor a completely clean build there's also quite a noticeable difference so this would also reduce CI times a bit. Measuring with cargo clean; time TESTNAME=___ cargo uitestOn It also shaves 2.2GB from the |
Centri3
left a comment
There was a problem hiding this comment.
I already made my opinions clear, but I'm 100% in favor of this. It's a bit unfortunate that we can't use Cargo.toml but it really makes testing a ton more time consuming.
|
Thanks! I will test this also in the rustc repo after the sync, to make sure that it also works out there. |
oli-obk
left a comment
There was a problem hiding this comment.
Sad, but makes sense. This also fixes rust-lang/rust#113585, so it may be good to do a sync to rustc quickly afterwards
|
Sync is tomorrow, so lets get this in. @bors r+ |
|
re: |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
changelog: none This tries to make progress on rust-lang/rust#78717 by using the ui_test dependency handling instead of linking in the dependencies of clippy itself with the tests. This partially reverts #11045. However, we still use the old style of dealing with dependencies for clippy's own crates and the "internal" tests, as otherwise those would get rebuilt which takes too long.
changelog: none
context: https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/Building.20test.20dependencies
This restores the old
EXTERN_FLAGSmethod of passing--externflags for building UI tests with minor changesVecof args instead of a command stringBTreeMapso the extern flags are in alphabetical order and deterministicI don't know if the
HOST_LIBSpart is still required, but I figured it best to leave it in for now. If the change is accepted we can take a look if it's needed inrust-lang/rustafter the next syncThis isn't as pleasant as having a
Cargo.toml, though there is something satisfying about knowing the dependencies are already built and not needing to invokecargor? @flip1995