-
Notifications
You must be signed in to change notification settings - Fork 61
difftest: nextest support and speedups #334
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
Open
Firestar99
wants to merge
14
commits into
main
Choose a base branch
from
difftest_refactor
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
78127d8
difftest: use relative paths, create output directory in target
Firestar99 7ccc509
difftest: fix test mod paths with subdirectories
Firestar99 c1c743b
difftest: fix test names on windows
Firestar99 6b2c547
difftest: move difftest `bin` to `runner`
Firestar99 7fc13f9
difftest: switch to `mimic-libtest` from `tester`
Firestar99 d8673ff
difftest: disable logger when listing available tests
Firestar99 83516cd
difftest: fix `use-installed-tools` feature
Firestar99 64ab6a1
difftest: use nextest to properly filter tests, adjust ci
Firestar99 6d01e31
difftest: move `lib` into `tests` workspace, move `mod config` to new…
Firestar99 15ecef1
difftest: compile difftest in ci before running
Firestar99 3bfc9bc
ci: do not fail-fast matrices
Firestar99 b72b523
difftest: better io error reporting in config
Firestar99 fe29be2
difftest: cargo windows bug workaround, mirror all workspace dependen…
Firestar99 5114309
difftest: Unconditionally create output file
LegNeato File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
[profile.default] | ||
default-filter = '!package(difftest*) & !package(compiletest*) & !package(example-runner-*)' | ||
fail-fast = false | ||
|
||
[profile.difftests] | ||
default-filter = 'package(difftests)' | ||
slow-timeout = "2m" | ||
|
||
[profile.difftest-runner] | ||
default-filter = 'package(difftest-runner) | package(difftest-types)' |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My main concern here is we are making CI smarter when best practice is to make it as dumb as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still a little unsure what to do about precompiling the tests, which is not just an CI issue but also when building from your own machine. Currently, it'll look like the tests are just stuck eating CPU for minutes, apart from the occasional slow warning from nextest, as all tests are blocked on the first test holding the target folder lock and compiling all dependencies. So at least for CI, instead of windows taking 7-8min of "nothing happening", we get cargo's compilation progress reporting.
We could move precompiling tests to the listing stage, but that would make
nextest list
take forever on first execution. Not even sure if we can even print compilation progress, as when listing tests we must only write a list of tests to stdout and nothing else, otherwise nextest will error out. Though I haven't tested if piping through stderr would work.Maybe we could alias
cargo difftest
to be proceeded by a test compile before running nextest?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't that basically what we had? It ran
cargo build --release
in the tests workspace regardless of what was running. The thinking was that most of the deps would be shared, but I guess I soon as I addedash
vswgpu
that went out the window.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Crates in a workspace will compile all their dependencies into the
target
dir of their workspace, regardless of where they're from. So on master, the difftestlib
crate would be compiled once in the difftest workspace's target and a second time in the root workspace's target. Now onlytypes
is shared like that, and a lot lighter since it's just plain types and nowgpu
orash
.