-
Notifications
You must be signed in to change notification settings - Fork 191
RUST-2281 Cache test binary for cross-topololgy tests #1549
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
base: main
Are you sure you want to change the base?
Conversation
| - name: check-cargo-deny | ||
| commands: | ||
| - func: "check cargo deny" | ||
| - command: shell.exec |
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.
These were tasks calling a function that itself was a single command and not used anywhere else; I got annoyed tracking down the indirections and collapsed them. I can revert this portion if you don't think it's an improvement :)
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.
Yeah I like this change - can these be subprocess.exec, though?
| - command: expansions.update | ||
| params: | ||
| file: src/uri-expansions.yml | ||
| # The server needs some time after startup otherwise it won't include things like |
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.
Turns out the time of the compilation step between mongo orchestration and running the tests was masking this behavior; took me a while to figure out why the relevant tests were failing.
| tags: [{version}, {top_name}] | ||
| depends_on: | ||
| - name: build-nextest-archive | ||
| patch_optional: true |
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.
Despite the name, what this actually means is "if the named task doesn't exist in this buildvariant, don't actually depend on it".
| /// let mut cs = coll.watch().session(&mut session).await?; | ||
| /// while let Some(event) = cs.next(&mut session).await? { | ||
| /// let id = bson::to_bson(&event.id)?; | ||
| /// let id = bson::serialize_to_bson(&event.id)?; |
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.
These failed because the new doctest target is running against bson3. I figure it's better to have the newer version be the one documented.
|
|
||
| # This file is not created by default on Windows | ||
| echo 'export PATH="$PATH:${CARGO_HOME}/bin"' >>${CARGO_HOME}/env | ||
| echo "export CARGO_NET_GIT_FETCH_WITH_CLI=true" >>${CARGO_HOME}/env |
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.
This was a fix for a years-old issue with git authentication on macos that's no longer a problem. Removed it because I was hitting very hard to diagnose issues with cargo not being able to locate the git binary on windows, likely something to do with cygwin path manipulation.
isabelatkinson
left a comment
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.
nice! just a small suggestion
| - name: check-cargo-deny | ||
| commands: | ||
| - func: "check cargo deny" | ||
| - command: shell.exec |
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.
Yeah I like this change - can these be subprocess.exec, though?
RUST-2281
This converts all of these to using cached binaries shared across the server topology tasks:
A lot of yaks to shave here:
cargo nextesthas support for this.cargo nextestdoesn't support doctests (they're apparently built in a different and hard to manage way); we were running those alongside the main tests but that doesn't work if the run is using an archive, so I've split that out into its own lint task.