mirror of
https://github.com/bahdotsh/wrkflw.git
synced 2026-09-01 19:50:23 +02:00
fix(executor): use stage-aware filtering for GitLab --job flag
It turns out that filter_plan_to_job walks `needs` edges to find transitive dependencies, which works fine for GitHub workflows. But GitLab pipelines use *stage ordering* for implicit dependencies, and convert_to_workflow_format sets `needs: None` on every converted job. So running `--job deploy` on a GitLab pipeline would silently drop all build and test jobs. Not great. Add filter_plan_to_job_by_stage that understands the GitLab model: keep all jobs in earlier stage batches (they're implicit deps) and filter only the target's own batch down to just the target job. The GitHub workflow path continues using the needs-based filter. While at it, extract the job-not-found error into a shared helper and add proper test coverage: 6 unit tests for the stage-aware filter plus 3 integration tests exercising the full execute_workflow path with target_job set.
This commit is contained in:
@@ -154,18 +154,7 @@ pub fn filter_plan_to_job(
|
||||
kind: &str,
|
||||
) -> Result<Vec<Vec<String>>, String> {
|
||||
if !jobs.contains_key(target_job) {
|
||||
let mut available: Vec<&String> = jobs.keys().collect();
|
||||
available.sort();
|
||||
return Err(format!(
|
||||
"Job '{}' not found in {}. Available jobs: {}",
|
||||
target_job,
|
||||
kind,
|
||||
available
|
||||
.iter()
|
||||
.map(|s| s.as_str())
|
||||
.collect::<Vec<_>>()
|
||||
.join(", ")
|
||||
));
|
||||
return Err(job_not_found_error(target_job, jobs, kind));
|
||||
}
|
||||
|
||||
let needed = collect_transitive_deps(target_job, jobs);
|
||||
@@ -182,6 +171,52 @@ pub fn filter_plan_to_job(
|
||||
.collect())
|
||||
}
|
||||
|
||||
/// Filter a stage-ordered execution plan to only include the target job and all
|
||||
/// jobs in preceding stages (implicit dependencies). This is appropriate for
|
||||
/// GitLab CI/CD where stage ordering defines implicit dependencies — all jobs in
|
||||
/// earlier stages must complete before later stages run.
|
||||
///
|
||||
/// In the target job's own stage batch, only the target job is kept; all earlier
|
||||
/// stage batches are preserved in full.
|
||||
pub fn filter_plan_to_job_by_stage(
|
||||
plan: Vec<Vec<String>>,
|
||||
target_job: &str,
|
||||
jobs: &HashMap<String, Job>,
|
||||
kind: &str,
|
||||
) -> Result<Vec<Vec<String>>, String> {
|
||||
if !jobs.contains_key(target_job) {
|
||||
return Err(job_not_found_error(target_job, jobs, kind));
|
||||
}
|
||||
|
||||
let mut result = Vec::new();
|
||||
for batch in plan {
|
||||
if batch.contains(&target_job.to_string()) {
|
||||
// Target's stage: only keep the target job itself
|
||||
result.push(vec![target_job.to_string()]);
|
||||
break;
|
||||
}
|
||||
// Earlier stage: keep all jobs (implicit dependencies)
|
||||
result.push(batch);
|
||||
}
|
||||
|
||||
Ok(result)
|
||||
}
|
||||
|
||||
fn job_not_found_error(target_job: &str, jobs: &HashMap<String, Job>, kind: &str) -> String {
|
||||
let mut available: Vec<&String> = jobs.keys().collect();
|
||||
available.sort();
|
||||
format!(
|
||||
"Job '{}' not found in {}. Available jobs: {}",
|
||||
target_job,
|
||||
kind,
|
||||
available
|
||||
.iter()
|
||||
.map(|s| s.as_str())
|
||||
.collect::<Vec<_>>()
|
||||
.join(", ")
|
||||
)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -346,4 +381,127 @@ mod tests {
|
||||
let filtered = filter_plan_to_job(plan, "a", &jobs, "workflow").unwrap();
|
||||
assert_eq!(filtered, vec![vec!["a".to_string()]]);
|
||||
}
|
||||
|
||||
// --- filter_plan_to_job_by_stage tests (GitLab stage-based filtering) ---
|
||||
|
||||
#[test]
|
||||
fn test_filter_by_stage_not_found() {
|
||||
let jobs = HashMap::new();
|
||||
let plan = vec![vec!["a".to_string()]];
|
||||
|
||||
let result = filter_plan_to_job_by_stage(plan, "missing", &jobs, "pipeline");
|
||||
assert!(result.is_err());
|
||||
let err = result.unwrap_err();
|
||||
assert!(err.contains("missing"));
|
||||
assert!(err.contains("pipeline"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_filter_by_stage_target_in_first_stage() {
|
||||
let mut jobs = HashMap::new();
|
||||
jobs.insert("build".to_string(), job_with_needs(None));
|
||||
jobs.insert("lint".to_string(), job_with_needs(None));
|
||||
jobs.insert("test".to_string(), job_with_needs(None));
|
||||
jobs.insert("deploy".to_string(), job_with_needs(None));
|
||||
|
||||
// Stages: [build, lint] -> [test] -> [deploy]
|
||||
let plan = vec![
|
||||
vec!["build".to_string(), "lint".to_string()],
|
||||
vec!["test".to_string()],
|
||||
vec!["deploy".to_string()],
|
||||
];
|
||||
|
||||
let filtered = filter_plan_to_job_by_stage(plan, "build", &jobs, "pipeline").unwrap();
|
||||
// Only the first stage, filtered to just "build"
|
||||
assert_eq!(filtered, vec![vec!["build".to_string()]]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_filter_by_stage_target_in_middle_stage() {
|
||||
let mut jobs = HashMap::new();
|
||||
jobs.insert("build".to_string(), job_with_needs(None));
|
||||
jobs.insert("lint".to_string(), job_with_needs(None));
|
||||
jobs.insert("test".to_string(), job_with_needs(None));
|
||||
jobs.insert("deploy".to_string(), job_with_needs(None));
|
||||
|
||||
// Stages: [build, lint] -> [test] -> [deploy]
|
||||
let plan = vec![
|
||||
vec!["build".to_string(), "lint".to_string()],
|
||||
vec!["test".to_string()],
|
||||
vec!["deploy".to_string()],
|
||||
];
|
||||
|
||||
let filtered = filter_plan_to_job_by_stage(plan, "test", &jobs, "pipeline").unwrap();
|
||||
// Keep all of stage 1, then just "test" from stage 2, drop stage 3
|
||||
assert_eq!(
|
||||
filtered,
|
||||
vec![
|
||||
vec!["build".to_string(), "lint".to_string()],
|
||||
vec!["test".to_string()],
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_filter_by_stage_target_in_last_stage() {
|
||||
let mut jobs = HashMap::new();
|
||||
jobs.insert("build".to_string(), job_with_needs(None));
|
||||
jobs.insert("test".to_string(), job_with_needs(None));
|
||||
jobs.insert("deploy".to_string(), job_with_needs(None));
|
||||
|
||||
// Stages: [build] -> [test] -> [deploy]
|
||||
let plan = vec![
|
||||
vec!["build".to_string()],
|
||||
vec!["test".to_string()],
|
||||
vec!["deploy".to_string()],
|
||||
];
|
||||
|
||||
let filtered = filter_plan_to_job_by_stage(plan, "deploy", &jobs, "pipeline").unwrap();
|
||||
assert_eq!(
|
||||
filtered,
|
||||
vec![
|
||||
vec!["build".to_string()],
|
||||
vec!["test".to_string()],
|
||||
vec!["deploy".to_string()],
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_filter_by_stage_filters_peers_in_target_stage() {
|
||||
let mut jobs = HashMap::new();
|
||||
jobs.insert("a".to_string(), job_with_needs(None));
|
||||
jobs.insert("b".to_string(), job_with_needs(None));
|
||||
jobs.insert("c".to_string(), job_with_needs(None));
|
||||
|
||||
// All in same stage: [a, b, c]
|
||||
let plan = vec![vec!["a".to_string(), "b".to_string(), "c".to_string()]];
|
||||
|
||||
let filtered = filter_plan_to_job_by_stage(plan, "b", &jobs, "pipeline").unwrap();
|
||||
// Only the target job from its stage
|
||||
assert_eq!(filtered, vec![vec!["b".to_string()]]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_filter_by_stage_drops_later_stages() {
|
||||
let mut jobs = HashMap::new();
|
||||
jobs.insert("compile".to_string(), job_with_needs(None));
|
||||
jobs.insert("unit_test".to_string(), job_with_needs(None));
|
||||
jobs.insert("integration_test".to_string(), job_with_needs(None));
|
||||
jobs.insert("deploy_staging".to_string(), job_with_needs(None));
|
||||
jobs.insert("deploy_prod".to_string(), job_with_needs(None));
|
||||
|
||||
// Stages: [compile] -> [unit_test, integration_test] -> [deploy_staging, deploy_prod]
|
||||
let plan = vec![
|
||||
vec!["compile".to_string()],
|
||||
vec!["unit_test".to_string(), "integration_test".to_string()],
|
||||
vec!["deploy_staging".to_string(), "deploy_prod".to_string()],
|
||||
];
|
||||
|
||||
let filtered = filter_plan_to_job_by_stage(plan, "unit_test", &jobs, "pipeline").unwrap();
|
||||
assert_eq!(
|
||||
filtered,
|
||||
vec![vec!["compile".to_string()], vec!["unit_test".to_string()],]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -221,10 +221,16 @@ async fn execute_gitlab_pipeline(
|
||||
// 3. Resolve job dependencies based on stages
|
||||
let execution_plan = resolve_gitlab_dependencies(&pipeline, &workflow)?;
|
||||
|
||||
// Filter to target job and its transitive dependencies if specified
|
||||
// Filter to target job and its stage-based dependencies if specified.
|
||||
// GitLab uses stages for implicit ordering, so we keep all earlier stages.
|
||||
let execution_plan = if let Some(ref target_job) = config.target_job {
|
||||
dependency::filter_plan_to_job(execution_plan, target_job, &workflow.jobs, "pipeline")
|
||||
.map_err(ExecutionError::Execution)?
|
||||
dependency::filter_plan_to_job_by_stage(
|
||||
execution_plan,
|
||||
target_job,
|
||||
&workflow.jobs,
|
||||
"pipeline",
|
||||
)
|
||||
.map_err(ExecutionError::Execution)?
|
||||
} else {
|
||||
execution_plan
|
||||
};
|
||||
|
||||
141
crates/wrkflw/tests/target_job_test.rs
Normal file
141
crates/wrkflw/tests/target_job_test.rs
Normal file
@@ -0,0 +1,141 @@
|
||||
use std::fs;
|
||||
use tempfile::tempdir;
|
||||
use wrkflw_lib::executor::engine::{execute_workflow, ExecutionConfig, RuntimeType};
|
||||
|
||||
fn write_file(path: &std::path::Path, content: &str) {
|
||||
fs::write(path, content).expect("failed to write file");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_target_job_runs_only_specified_job() {
|
||||
let dir = tempdir().unwrap();
|
||||
let workflow_path = dir.path().join("ci.yml");
|
||||
|
||||
let workflow = r#"
|
||||
name: CI
|
||||
on: push
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo "building"
|
||||
test:
|
||||
runs-on: ubuntu-latest
|
||||
needs: [build]
|
||||
steps:
|
||||
- run: echo "testing"
|
||||
deploy:
|
||||
runs-on: ubuntu-latest
|
||||
needs: [test]
|
||||
steps:
|
||||
- run: echo "deploying"
|
||||
"#;
|
||||
write_file(&workflow_path, workflow);
|
||||
|
||||
// Run only the "test" job — should include "build" (dependency) and "test",
|
||||
// but NOT "deploy".
|
||||
let cfg = ExecutionConfig {
|
||||
runtime_type: RuntimeType::Emulation,
|
||||
verbose: false,
|
||||
preserve_containers_on_failure: false,
|
||||
secrets_config: None,
|
||||
show_action_messages: false,
|
||||
target_job: Some("test".to_string()),
|
||||
};
|
||||
|
||||
let result = execute_workflow(&workflow_path, cfg)
|
||||
.await
|
||||
.expect("workflow execution failed");
|
||||
|
||||
let job_names: Vec<&str> = result.jobs.iter().map(|j| j.name.as_str()).collect();
|
||||
assert!(
|
||||
job_names.contains(&"build"),
|
||||
"expected build as a dependency"
|
||||
);
|
||||
assert!(job_names.contains(&"test"), "expected target job test");
|
||||
assert!(
|
||||
!job_names.contains(&"deploy"),
|
||||
"deploy should not run when targeting test"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_target_job_not_found_returns_error() {
|
||||
let dir = tempdir().unwrap();
|
||||
let workflow_path = dir.path().join("ci.yml");
|
||||
|
||||
let workflow = r#"
|
||||
name: CI
|
||||
on: push
|
||||
jobs:
|
||||
build:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo "building"
|
||||
"#;
|
||||
write_file(&workflow_path, workflow);
|
||||
|
||||
let cfg = ExecutionConfig {
|
||||
runtime_type: RuntimeType::Emulation,
|
||||
verbose: false,
|
||||
preserve_containers_on_failure: false,
|
||||
secrets_config: None,
|
||||
show_action_messages: false,
|
||||
target_job: Some("nonexistent".to_string()),
|
||||
};
|
||||
|
||||
let result = execute_workflow(&workflow_path, cfg).await;
|
||||
match result {
|
||||
Err(err) => {
|
||||
let msg = format!("{}", err);
|
||||
assert!(
|
||||
msg.contains("nonexistent"),
|
||||
"error should mention the job name"
|
||||
);
|
||||
}
|
||||
Ok(_) => panic!("expected error for nonexistent job"),
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_target_job_with_no_deps_runs_alone() {
|
||||
let dir = tempdir().unwrap();
|
||||
let workflow_path = dir.path().join("ci.yml");
|
||||
|
||||
let workflow = r#"
|
||||
name: CI
|
||||
on: push
|
||||
jobs:
|
||||
lint:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo "linting"
|
||||
test:
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- run: echo "testing"
|
||||
deploy:
|
||||
runs-on: ubuntu-latest
|
||||
needs: [lint, test]
|
||||
steps:
|
||||
- run: echo "deploying"
|
||||
"#;
|
||||
write_file(&workflow_path, workflow);
|
||||
|
||||
// Target "lint" which has no dependencies — only lint should run
|
||||
let cfg = ExecutionConfig {
|
||||
runtime_type: RuntimeType::Emulation,
|
||||
verbose: false,
|
||||
preserve_containers_on_failure: false,
|
||||
secrets_config: None,
|
||||
show_action_messages: false,
|
||||
target_job: Some("lint".to_string()),
|
||||
};
|
||||
|
||||
let result = execute_workflow(&workflow_path, cfg)
|
||||
.await
|
||||
.expect("workflow execution failed");
|
||||
|
||||
assert_eq!(result.jobs.len(), 1, "only the target job should run");
|
||||
assert_eq!(result.jobs[0].name, "lint");
|
||||
}
|
||||
Reference in New Issue
Block a user