Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions e2e/tasks/test_task_raw_output
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/usr/bin/env bash

# Test that raw tasks get interleave output even when task_output=prefix is set
# This ensures raw=true takes priority over task_output setting

cat <<EOF >mise.toml
[settings]
task_output = "prefix"

[tasks.normal]
run = 'echo normal task'

[tasks.raw_task]
raw = true
run = 'echo raw task output'
EOF

# Normal task should use prefix output (from settings)
assert_contains "mise run normal" "[normal] normal task"

# Raw task should get interleave output mode, meaning stdout goes directly
# without the [task_name] prefix being applied to the actual output
assert "mise run raw_task" "raw task output"

# Verify stdout doesn't have the prefix (only check stdout, not stderr)
stdout_output=$(mise run raw_task 2>/dev/null)
if [[ $stdout_output == *"[raw_task]"* ]]; then
echo "FAIL: raw task stdout should not have prefix"
exit 1
fi
echo "SUCCESS: raw task stdout has no prefix"
172 changes: 171 additions & 1 deletion src/task/task_output_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,21 @@ impl OutputHandler {
return TaskOutput::Quiet;
}

// CLI flags (--prefix, --interleave) override config settings
if self.prefix {
TaskOutput::Prefix
} else if self.interleave {
TaskOutput::Interleave
} else if let Some(output) = Settings::get().task_output {
output
// Silent/quiet from config override raw (output suppression takes precedence)
// Other modes (prefix, etc.) allow raw to take precedence for stdin/stdout
if output.is_silent() || output.is_quiet() {
output
} else if self.raw(task) {
TaskOutput::Interleave
} else {
output
}
} else if self.raw(task) || self.jobs() == 1 || self.is_linear {
TaskOutput::Interleave
} else {
Expand Down Expand Up @@ -171,3 +180,164 @@ impl OutputHandler {
}
}
}

#[cfg(test)]
mod tests {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think just remove this and rely on the e2e test unless you think it's adding value

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good to me 👍

use super::*;
use crate::config::settings::SettingsPartial;
use confique::Partial;

// Mutex to ensure tests don't interfere with each other when modifying global settings
static TEST_SETTINGS_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());

// Helper to run test with default settings (no task_output set)
// Acquires lock and resets settings to prevent race conditions with other tests
fn with_default_settings<F, R>(test_fn: F) -> R
where
F: FnOnce() -> R,
{
let _guard = TEST_SETTINGS_LOCK.lock().unwrap();

let mut settings = SettingsPartial::empty();
settings.silent = Some(false);
settings.quiet = Some(false);
settings.raw = Some(false);

crate::config::Settings::reset(Some(settings));
let result = test_fn();
crate::config::Settings::reset(None);

result
}

// Helper to run test with specific task_output setting
// Also explicitly sets silent/quiet/raw to false to prevent env vars from affecting tests
fn with_task_output_setting<F, R>(task_output: TaskOutput, test_fn: F) -> R
where
F: FnOnce() -> R,
{
let _guard = TEST_SETTINGS_LOCK.lock().unwrap();

let mut settings = SettingsPartial::empty();
settings.task_output = Some(task_output);
// Explicitly set these to prevent environment variables from affecting tests
settings.silent = Some(false);
settings.quiet = Some(false);
settings.raw = Some(false);

crate::config::Settings::reset(Some(settings));
let result = test_fn();
crate::config::Settings::reset(None);

result
}

fn default_config() -> OutputHandlerConfig {
OutputHandlerConfig {
prefix: false,
interleave: false,
output: None,
silent: false,
quiet: false,
raw: false,
is_linear: false,
jobs: None,
}
}

fn raw_task() -> Task {
Task {
raw: true,
..Default::default()
}
}

#[test]
fn test_raw_task_gets_interleave_output() {
with_default_settings(|| {
let handler = OutputHandler::new(default_config());
let task = raw_task();
assert_eq!(handler.output(Some(&task)), TaskOutput::Interleave);
});
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Test race condition due to missing lock acquisition

The test_raw_task_gets_interleave_output test does not acquire TEST_SETTINGS_LOCK before calling handler.output(), which reads from global Settings. Other tests (like test_task_output_silent_applies_to_raw_task) do acquire the lock and modify global settings via Settings::reset(). Since Rust runs tests in parallel by default, if both tests run concurrently, the first test may read task_output = Silent set by the other test. In that case, output.is_silent() returns true at line 128, causing the function to return TaskOutput::Silent instead of the expected TaskOutput::Interleave, leading to intermittent test failures.

Fix in Cursor Fix in Web


#[test]
fn test_prefix_flag_overrides_raw() {
with_default_settings(|| {
let config = OutputHandlerConfig {
prefix: true,
..default_config()
};
let handler = OutputHandler::new(config);
let task = raw_task();
assert_eq!(handler.output(Some(&task)), TaskOutput::Prefix);
});
}

#[test]
fn test_raw_task_overrides_task_output_setting() {
// Key test: raw=true must override task_output=prefix setting
with_task_output_setting(TaskOutput::Prefix, || {
let handler = OutputHandler::new(default_config());
let task = raw_task();
assert_eq!(handler.output(Some(&task)), TaskOutput::Interleave);
});
}

#[test]
fn test_task_output_setting_applies_to_normal_tasks() {
with_task_output_setting(TaskOutput::Prefix, || {
let handler = OutputHandler::new(default_config());
let task = Task::default();
assert_eq!(handler.output(Some(&task)), TaskOutput::Prefix);
});
}

#[test]
fn test_task_output_silent_applies_to_raw_task() {
// task_output=silent should still apply to raw tasks (output suppression takes precedence)
with_task_output_setting(TaskOutput::Silent, || {
let handler = OutputHandler::new(default_config());
let task = raw_task();
assert_eq!(handler.output(Some(&task)), TaskOutput::Silent);
});
}

#[test]
fn test_task_output_quiet_applies_to_raw_task() {
// task_output=quiet should still apply to raw tasks (output suppression takes precedence)
with_task_output_setting(TaskOutput::Quiet, || {
let handler = OutputHandler::new(default_config());
let task = raw_task();
assert_eq!(handler.output(Some(&task)), TaskOutput::Quiet);
});
}

#[test]
fn test_cli_prefix_overrides_task_output_silent() {
// CLI --prefix flag should override task_output=silent from config
with_task_output_setting(TaskOutput::Silent, || {
let config = OutputHandlerConfig {
prefix: true,
..default_config()
};
let handler = OutputHandler::new(config);
let task = raw_task();
assert_eq!(handler.output(Some(&task)), TaskOutput::Prefix);
});
}

#[test]
fn test_cli_interleave_overrides_task_output_silent() {
// CLI --interleave flag should override task_output=silent from config
with_task_output_setting(TaskOutput::Silent, || {
let config = OutputHandlerConfig {
interleave: true,
..default_config()
};
let handler = OutputHandler::new(config);
let task = raw_task();
assert_eq!(handler.output(Some(&task)), TaskOutput::Interleave);
});
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Unit tests depend on global Settings

The new unit tests call handler.output(...), which consults Settings::get() (including env/config). If a developer or CI environment sets values like MISE_TASK_OUTPUT/raw/silent, these tests can fail or pass for the wrong reasons, making them flaky and environment-dependent.

Fix in Cursor Fix in Web

}
Loading