[nexus-test-utils] replace build.rs with a new setup script #4150
Conversation
Created using spr 1.3.4
Created using spr 1.3.4
nexus/test-utils/src/db.rs
Outdated
| // TODO: replace with temp dir written out by nextest | ||
| // Use "out/" for now. |
There was a problem hiding this comment.
Isn't this happening already?
There was a problem hiding this comment.
Isn't this happening already?
Yes, lol. Will fix.
| &ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Info }, | ||
| ); | ||
| let dir = ensure_seed_directory_exists(&logctx.log).await; | ||
| if let Ok(env_path) = std::env::var("NEXTEST_ENV") { |
There was a problem hiding this comment.
Out of curiosity, what sets this environment variable?
Also, if this environment variable cannot be found, should we throw an error?
There was a problem hiding this comment.
Out of curiosity, what sets this environment variable?
Nextest does -- it's the way setup scripts can communicate with nextest.
Also, if this environment variable cannot be found, should we throw an error?
I figured folks might want to run the script by hand. Maybe we should warn in that case.
There was a problem hiding this comment.
Gotcha. So someone could run this binary manually, and it would create the seed dir, but not pass the necessary info to nextest.
That makes sense -- I'm okay with this structure as-is, I just wanted to make sure it would be clear "where things went wrong" if the environment variable from nextest was not provided.
There was a problem hiding this comment.
I added some more logging and a warning message to make this clearer.
crdb-seed/src/main.rs
Outdated
| // If the directory didn't exist when we started, try to create it. | ||
| // | ||
| // Note that this may be executed concurrently by many tests, so | ||
| // we should consider it possible for another caller to create this | ||
| // seed directory before we finish setting it up ourselves. |
There was a problem hiding this comment.
Is this still true, or will nextest prevent the concurrent execution of this function?
There was a problem hiding this comment.
Nextest will execute it once, but it's possible that the user spins up two instances of nextest at the same time. I'll update the comment.
Created using spr 1.3.4
sunshowers
deleted the
sunshowers/spr/nexus-test-utils-replace-buildrs-with-a-new-setup-script
branch
Nextest now has support for setup scripts, which means that we can
replace the existing
build.rswith a small binary.I also took the liberty of switching some of the related code over to
camino.
Supersedes #4056.