-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(task): resolve vars from subdirectory configs for monorepo tasks #8343
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| #!/usr/bin/env bash | ||
| # Test that monorepo tasks properly load [vars] from subdirectory configs | ||
| export MISE_EXPERIMENTAL=1 | ||
|
|
||
| # Create monorepo root config with vars | ||
| cat <<EOF >mise.toml | ||
| experimental_monorepo_root = true | ||
|
|
||
| [monorepo] | ||
| config_roots = ["infra/stacks/*"] | ||
|
|
||
| [vars] | ||
| ROOT_VAR = "root_value" | ||
| SHARED_VAR = "from_root" | ||
|
|
||
| [tasks.root-greet] | ||
| run = 'echo "ROOT_VAR={{vars.ROOT_VAR}} SHARED_VAR={{vars.SHARED_VAR}}"' | ||
| EOF | ||
|
|
||
| # Create subdirectory config with its own vars | ||
| mkdir -p infra/stacks/gcp | ||
| cat <<EOF >infra/stacks/gcp/mise.toml | ||
| [vars] | ||
| GCP_VAR = "gcp_value" | ||
| SHARED_VAR = "from_gcp" | ||
|
|
||
| [tasks.greet] | ||
| run = 'echo "GCP_VAR={{vars.GCP_VAR}} SHARED_VAR={{vars.SHARED_VAR}} ROOT_VAR={{vars.ROOT_VAR}}"' | ||
| EOF | ||
|
|
||
| # Create MISE_ENV-specific config for the subdirectory | ||
| cat <<EOF >infra/stacks/gcp/mise.dev.toml | ||
| [vars] | ||
| DEV_VAR = "dev_value" | ||
| SHARED_VAR = "from_gcp_dev" | ||
| EOF | ||
|
|
||
| # Test 1: Root task sees root vars | ||
| echo "=== Test 1: Root task sees root vars ===" | ||
| unset MISE_ENV | ||
| output=$(mise run //:root-greet) | ||
| echo "$output" | ||
| assert_contains "echo '$output'" "ROOT_VAR=root_value" | ||
| assert_contains "echo '$output'" "SHARED_VAR=from_root" | ||
|
|
||
| # Test 2: Subdirectory task sees subdirectory vars (the core bug fix) | ||
| echo "=== Test 2: Subdirectory task sees subdirectory vars ===" | ||
| unset MISE_ENV | ||
| output=$(mise run '//infra/stacks/gcp:greet') | ||
| echo "$output" | ||
| assert_contains "echo '$output'" "GCP_VAR=gcp_value" | ||
| assert_contains "echo '$output'" "SHARED_VAR=from_gcp" | ||
|
|
||
| # Test 3: Subdirectory task still inherits root vars it doesn't override | ||
| echo "=== Test 3: Subdirectory task inherits root vars ===" | ||
| unset MISE_ENV | ||
| output=$(mise run '//infra/stacks/gcp:greet') | ||
| echo "$output" | ||
| assert_contains "echo '$output'" "ROOT_VAR=root_value" | ||
|
|
||
| # Test 4: Subdirectory vars override root vars of the same name | ||
| echo "=== Test 4: Subdirectory vars override root vars ===" | ||
| unset MISE_ENV | ||
| output=$(mise run '//infra/stacks/gcp:greet') | ||
| echo "$output" | ||
| # SHARED_VAR should be from_gcp, not from_root | ||
| assert_contains "echo '$output'" "SHARED_VAR=from_gcp" | ||
|
|
||
| # Test 5: Subdirectory task with MISE_ENV sees env-specific vars | ||
| echo "=== Test 5: MISE_ENV-specific vars in subdirectory ===" | ||
| # Add a task that uses the DEV_VAR | ||
| cat <<EOF >infra/stacks/gcp/mise.toml | ||
| [vars] | ||
| GCP_VAR = "gcp_value" | ||
| SHARED_VAR = "from_gcp" | ||
|
|
||
| [tasks.greet] | ||
| run = 'echo "GCP_VAR={{vars.GCP_VAR}} SHARED_VAR={{vars.SHARED_VAR}} ROOT_VAR={{vars.ROOT_VAR}}"' | ||
|
|
||
| [tasks.greet-dev] | ||
| run = 'echo "DEV_VAR={{vars.DEV_VAR}} SHARED_VAR={{vars.SHARED_VAR}}"' | ||
| EOF | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of overwriting the entire |
||
|
|
||
| output=$(MISE_ENV=dev mise run '//infra/stacks/gcp:greet-dev') | ||
| echo "$output" | ||
| assert_contains "echo '$output'" "DEV_VAR=dev_value" | ||
| # MISE_ENV vars should override base vars | ||
| assert_contains "echo '$output'" "SHARED_VAR=from_gcp_dev" | ||
|
|
||
| echo "=== All tests passed! ===" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -691,6 +691,17 @@ impl Task { | |
| config: &Arc<Config>, | ||
| cwd: Option<PathBuf>, | ||
| env: &EnvMap, | ||
| ) -> Result<(usage::Spec, Vec<String>)> { | ||
| self.parse_usage_spec_with_vars(config, cwd, env, None) | ||
| .await | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| pub async fn parse_usage_spec_with_vars( | ||
| &self, | ||
| config: &Arc<Config>, | ||
| cwd: Option<PathBuf>, | ||
| env: &EnvMap, | ||
| extra_vars: Option<IndexMap<String, String>>, | ||
| ) -> Result<(usage::Spec, Vec<String>)> { | ||
| let (mut spec, scripts) = if let Some(file) = self.file_path(config).await? { | ||
| let spec = usage::Spec::parse_script(&file) | ||
|
|
@@ -704,7 +715,11 @@ impl Task { | |
| (spec, vec![]) | ||
| } else { | ||
| let scripts_only = self.run_script_strings(); | ||
| let (scripts, spec) = TaskScriptParser::new(cwd) | ||
| let mut parser = TaskScriptParser::new(cwd); | ||
| if let Some(vars) = extra_vars { | ||
| parser = parser.with_extra_vars(vars); | ||
| } | ||
| let (scripts, spec) = parser | ||
| .parse_run_scripts(config, self, &scripts_only, env) | ||
| .await?; | ||
| (spec, scripts) | ||
|
|
@@ -741,11 +756,18 @@ impl Task { | |
| cwd: Option<PathBuf>, | ||
| args: &[String], | ||
| env: &EnvMap, | ||
| extra_vars: Option<IndexMap<String, String>>, | ||
| ) -> Result<Vec<(String, Vec<String>)>> { | ||
| let (spec, scripts) = self.parse_usage_spec(config, cwd.clone(), env).await?; | ||
| let (spec, scripts) = self | ||
| .parse_usage_spec_with_vars(config, cwd.clone(), env, extra_vars.clone()) | ||
| .await?; | ||
| if has_any_args_defined(&spec) { | ||
| let scripts_only = self.run_script_strings(); | ||
| let scripts = TaskScriptParser::new(cwd) | ||
| let mut parser = TaskScriptParser::new(cwd); | ||
| if let Some(vars) = extra_vars { | ||
| parser = parser.with_extra_vars(vars); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| let scripts = parser | ||
| .parse_run_scripts_with_args(config, self, &scripts_only, env, args, &spec) | ||
| .await?; | ||
| Ok(scripts.into_iter().map(|s| (s, vec![])).collect()) | ||
|
|
||
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.
Tests 2, 3, and 4 each run the same
mise runcommand. For better efficiency and clarity, you could combine these into a single test case that runs the command once and then performs all related assertions.