Skip to content

Commit

Permalink
test: Create repos in LimmatChildBuilder::new
Browse files Browse the repository at this point in the history
Now that the buider can be reused and shares the repo by default this
can be simplified. This also incidentally fixes the bug where
`LimmatChild::result_exists` creates a new repo every time, which
seems to sometimes cause the executor to get starved and fail to
actually spawn the child.

Kinda disappointing coz I hoped maybe this would unearth the perf
issues I've been seeing on my work computer. But it's just a dumb
test harness thing. Oh well.
  • Loading branch information
bjackman committed Dec 24, 2024
1 parent a72e6ae commit 7b86c7b
Showing 1 changed file with 13 additions and 36 deletions.
49 changes: 13 additions & 36 deletions tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ impl ChildExt for Child {

struct LimmatChildBuilder {
temp_dir: TempDir,
existing_repo_dir: Option<PathBuf>, // If None, create one right in temp_dir.
repo_dir: PathBuf,
db_dir: PathBuf,
dump_output_on_panic: bool,
env: HashMap<OsString, OsString>,
Expand All @@ -78,11 +78,15 @@ impl<'a> LimmatChildBuilder {
async fn new(config: impl AsRef<str>) -> anyhow::Result<Self> {
let temp_dir = TempDir::with_prefix("limmat-child")?;

let repo_dir = temp_dir.path().join("repo");
create_dir(&repo_dir).unwrap();
Self::init_test_repo(&repo_dir).await.unwrap();

let db_dir = temp_dir.path().join("cache");
create_dir(&db_dir).unwrap();
Ok(Self {
temp_dir,
existing_repo_dir: None,
repo_dir,
db_dir,
dump_output_on_panic: true,
env: HashMap::from([("RUST_LOG".into(), "debug".into())]),
Expand All @@ -95,11 +99,6 @@ impl<'a> LimmatChildBuilder {
self
}

fn existing_repo_dir(mut self, dir: PathBuf) -> Self {
self.existing_repo_dir = Some(dir);
self
}

fn dump_output_on_panic(mut self, dump: bool) -> Self {
self.dump_output_on_panic = dump;
self
Expand All @@ -111,6 +110,7 @@ impl<'a> LimmatChildBuilder {
}

async fn init_test_repo(path: &Path) -> anyhow::Result<()> {
eprintln!("initializing test repo");
Command::new("/usr/bin/git")
.stderr(Stdio::null())
.stdout(Stdio::null())
Expand All @@ -120,7 +120,8 @@ impl<'a> LimmatChildBuilder {
.await?
.check_exit_ok()
.context("git init")?;
for _ in 0..5 {
for i in 0..5 {
eprintln!(" commit {i}");
Command::new("/usr/bin/git")
.stderr(Stdio::null())
.stdout(Stdio::null())
Expand All @@ -141,21 +142,13 @@ impl<'a> LimmatChildBuilder {
let stderr = File::create(self.temp_dir.path().join("stderr.txt"))?;
let stdout = File::create(self.temp_dir.path().join("stdout.txt"))?;

let repo_dir = match self.existing_repo_dir {
Some(ref dir) => dir,
None => {
Self::init_test_repo(self.temp_dir.path()).await?;
self.temp_dir.path()
}
};

let mut cmd: Command = get_test_bin("limmat").into();
let cmd = cmd
.args([
"--config",
"/dev/stdin",
"--repo",
repo_dir.to_str().unwrap(),
self.repo_dir.to_str().unwrap(),
"--result-db",
self.db_dir.to_str().unwrap(),
"--worktree-dir",
Expand Down Expand Up @@ -506,10 +499,6 @@ async fn should_run_test(config: &str) {
#[googletest::test]
#[tokio::test]
async fn should_run_test_with_stored_results() {
let repo_dir = TempDir::with_prefix("repo").unwrap();
LimmatChildBuilder::init_test_repo(repo_dir.path())
.await
.unwrap();
let temp_dir = TempDir::new().unwrap();

let signalling_path = temp_dir.path().join("trans-output.txt");
Expand Down Expand Up @@ -548,8 +537,7 @@ async fn should_run_test_with_stored_results() {
let builder = LimmatChildBuilder::new(&config)
.await
.unwrap()
.db_dir(db_dir.path().to_owned())
.existing_repo_dir(repo_dir.path().to_owned());
.db_dir(db_dir.path().to_owned());
let mut child = builder.start(["test", "my_other_dep"]).await.unwrap();
timeout(Duration::from_secs(5), child.expect_success())
.await
Expand Down Expand Up @@ -582,11 +570,6 @@ async fn should_run_test_with_stored_results() {
#[googletest::test]
#[tokio::test]
async fn should_find_output(want_stdout: &str, want_stderr: &str) {
let repo_dir = TempDir::with_prefix("repo").unwrap();
LimmatChildBuilder::init_test_repo(repo_dir.path())
.await
.unwrap();

let db_dir = TempDir::with_prefix("result-db").unwrap();
let builder = LimmatChildBuilder::new(
r##"
Expand All @@ -604,8 +587,7 @@ async fn should_find_output(want_stdout: &str, want_stderr: &str) {
)
.await
.unwrap()
.db_dir(db_dir.path().to_owned())
.existing_repo_dir(repo_dir.path().to_owned());
.db_dir(db_dir.path().to_owned());
let mut child = builder
.start(["get", "--run", "my_test", "HEAD^"])
.await
Expand Down Expand Up @@ -648,11 +630,7 @@ async fn should_find_output(want_stdout: &str, want_stderr: &str) {
#[googletest::test]
#[tokio::test]
async fn should_find_not_race() {
let repo_dir = TempDir::with_prefix("repo").unwrap();
let tmp_dir = TempDir::new().unwrap();
LimmatChildBuilder::init_test_repo(repo_dir.path())
.await
.unwrap();
let db_dir = TempDir::with_prefix("result-db").unwrap();

let builder = LimmatChildBuilder::new(format!(
Expand All @@ -669,8 +647,7 @@ async fn should_find_not_race() {
.unwrap()
.db_dir(db_dir.path().to_owned())
// Hack - this is making for annoying log spam when it fails at the moment.
.dump_output_on_panic(false)
.existing_repo_dir(repo_dir.path().to_owned());
.dump_output_on_panic(false);
let mut watch_children = Vec::new();
for _ in 0..16 {
let child = builder.start(["watch", "HEAD^"]).await.unwrap();
Expand Down

0 comments on commit 7b86c7b

Please sign in to comment.