Skip to content

Commit

Permalink
fix: pass validation when number of persistent tasks equals concurrency
Browse files Browse the repository at this point in the history
  • Loading branch information
Alexander Lyon authored and Alexander Lyon committed Jan 8, 2024
1 parent 92b2472 commit 7699d7c
Showing 1 changed file with 110 additions and 1 deletion.
111 changes: 110 additions & 1 deletion crates/turborepo-lib/src/engine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ impl Engine<Built> {
(count, errs)
});

if persistent_count >= concurrency {
if persistent_count > concurrency {
validation_errors.push(ValidateError::PersistentTasksExceedConcurrency {
persistent_count,
concurrency,
Expand Down Expand Up @@ -273,3 +273,112 @@ impl fmt::Display for TaskNode {
}
}
}

#[cfg(test)]
mod test {

use std::collections::BTreeMap;

use tempdir::TempDir;
use turbopath::AbsoluteSystemPath;
use turborepo_repository::{
discovery::{DiscoveryResponse, PackageDiscovery, PackageDiscoveryBuilder, WorkspaceData},
package_graph::PackageGraphBuilder,
package_json::PackageJson,
};

use super::*;

struct DummyDiscovery<'a>(&'a TempDir);

impl<'a> PackageDiscovery for DummyDiscovery<'a> {
async fn discover_packages(
&mut self,
) -> Result<
turborepo_repository::discovery::DiscoveryResponse,
turborepo_repository::discovery::Error,
> {
// our workspace has three packages, two of which have a build script
let workspaces = [("a", true), ("b", true), ("c", false)]
.into_iter()
.map(|(name, had_build)| {
let path = AbsoluteSystemPath::from_std_path(self.0.path()).unwrap();
let package_json = path.join_component(&format!("{}.json", name));

let scripts = if had_build {
BTreeMap::from_iter(
[("build".to_string(), "echo built!".to_string())].into_iter(),
)
} else {
BTreeMap::default()
};

let package = PackageJson {
name: Some(name.to_string()),
scripts,
..Default::default()
};

let file = std::fs::File::create(package_json.as_std_path()).unwrap();
serde_json::to_writer(file, &package).unwrap();

WorkspaceData {
package_json,
turbo_json: None,
}
})
.collect();

Ok(DiscoveryResponse {
package_manager: turborepo_repository::package_manager::PackageManager::Pnpm,
workspaces,
})
}
}

#[tokio::test]
async fn issue_4291() {
// we had an issue where our engine validation would reject running persistent
// tasks if the number of _total packages_ exceeded the concurrency limit,
// rather than the number of package that had that task. in this test, we
// set up a workspace with three packages, two of which have a persistent build
// task. we expect concurrency limit 1 to fail, but 2 and 3 to pass.

let tmp = tempdir::TempDir::new("issue_4291").unwrap();

let mut engine = Engine::new();

// add two packages with a persistent build task
for package in ["a", "b"] {
let task_id = TaskId::new(package, "build");
engine.get_index(&task_id);
engine.add_definition(
task_id,
TaskDefinition {
persistent: true,
..Default::default()
},
);
}

let engine = engine.seal();
let current_dir = std::env::current_dir().unwrap();

let mut graph_builder = PackageGraph::builder(
AbsoluteSystemPath::from_std_path(&current_dir).unwrap(),
PackageJson::default(),
)
.with_package_discovery(DummyDiscovery(&tmp));

let graph = graph_builder.build().await.unwrap();

// if our limit is less than, it should fail
engine.validate(&graph, 1).expect_err("not enough");

// if out limit is equal, then it should pass, since the third has no build task
engine.validate(&graph, 2).expect("ok");

// if our limit is greater, then it should pass
engine.validate(&graph, 3).expect("ok");
}
}

0 comments on commit 7699d7c

Please sign in to comment.