Skip to content

Commit

Permalink
new: Improve task output prefixing.
Browse files Browse the repository at this point in the history
  • Loading branch information
milesj committed Sep 12, 2024
1 parent 59d3950 commit d5801ef
Show file tree
Hide file tree
Showing 18 changed files with 154 additions and 159 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#### 🚀 Updates

- Updated cached task outputs to now be prefixed with the task target when printing to the console.
- Updated Bun/Yarn lockfile parsing to temporarily log a warning on parse failure instead of exiting
with an error. This change was made as there are currently no actionable or human-readable error
messages.
Expand Down
9 changes: 9 additions & 0 deletions crates/action-context/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,15 @@ impl ActionContext {
mutex
}

pub fn get_target_prefix<T: AsRef<Target>>(&self, target: T) -> String {
target.as_ref().to_prefix(
self.primary_targets
.iter()
.map(|target| target.id.len())
.max(),
)
}

pub fn get_target_states(&self) -> FxHashMap<Target, TargetState> {
let mut map = FxHashMap::default();
self.target_states.scan(|k, v| {
Expand Down
2 changes: 1 addition & 1 deletion crates/cli/tests/run_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ fn disambiguates_same_tasks_with_diff_args_envs() {

// The order changes so we can't snapshot it
assert!(predicate::str::contains("taskDeps:base")
.count(8) // 4 start + 4 end
.count(11) // 4 start + 4 end + 3 output prefixes
.eval(&output));
assert!(predicate::str::contains("a b c").eval(&output));
assert!(predicate::str::contains("TEST_VAR=value").eval(&output));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ expression: assert.output()
---
▪▪▪▪ npm install
▪▪▪▪ outputStyles:buffer
stdout
outputStyles:buffer | stdout
▪▪▪▪ outputStyles:buffer (100ms)
▪▪▪▪ outputStyles:bufferPrimary (no op)

Tasks: 2 completed
Time: 100ms

stderr
outputStyles:buffer | stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ expression: assert.output()
---
▪▪▪▪ npm install
▪▪▪▪ outputStyles:stream
outputStyles:stream | stdout
outputStyles:stream | stdout
▪▪▪▪ outputStyles:stream (100ms)
▪▪▪▪ outputStyles:streamPrimary (no op)

Tasks: 2 completed
Time: 100ms

outputStyles:stream | stderr
outputStyles:stream | stderr
54 changes: 19 additions & 35 deletions crates/console-reporter/src/default_reporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,20 +102,26 @@ impl DefaultReporter {
operation: &Operation,
item: &TaskReportItem,
) -> miette::Result<()> {
let print_stdout = || -> miette::Result<()> {
let print = || -> miette::Result<()> {
if let Some(output) = operation.get_output() {
if let Some(out) = &output.stdout {
self.out.write_line(out.trim())?;
if !out.is_empty() {
if let Some(prefix) = &item.output_prefix {
self.out.write_line_with_prefix(out.trim(), prefix)?;
} else {
self.out.write_line(out.trim())?;
}
}
}
}

Ok(())
};

let print_stderr = || -> miette::Result<()> {
if let Some(output) = operation.get_output() {
if let Some(out) = &output.stderr {
self.err.write_line(out.trim())?;
if let Some(err) = &output.stderr {
if !err.is_empty() {
if let Some(prefix) = &item.output_prefix {
self.err.write_line_with_prefix(err.trim(), prefix)?;
} else {
self.err.write_line(err.trim())?;
}
}
}
}

Expand All @@ -126,8 +132,7 @@ impl DefaultReporter {
// Only show output on failure
Some(TaskOutputStyle::BufferOnlyFailure) => {
if operation.has_failed() {
print_stdout()?;
print_stderr()?;
print()?;
}
}
// Only show the hash
Expand All @@ -141,8 +146,7 @@ impl DefaultReporter {
Some(TaskOutputStyle::None) => {}
// Show output on both success and failure
_ => {
print_stdout()?;
print_stderr()?;
print()?;
}
};

Expand All @@ -157,26 +161,7 @@ impl DefaultReporter {

if let Some(attempt) = action.operations.get_last_execution() {
if attempt.has_failed() {
let mut has_stdout = false;

if let Some(output) = attempt.get_output() {
if let Some(stdout) = &output.stdout {
if !stdout.is_empty() {
has_stdout = true;
self.out.write_line(stdout.trim())?;
}
}

if let Some(stderr) = &output.stderr {
if has_stdout {
self.out.write_newline()?;
}

if !stderr.is_empty() {
self.out.write_line(stderr.trim())?;
}
}
}
self.print_operation_output(attempt, &TaskReportItem::default())?;
}
}

Expand Down Expand Up @@ -414,7 +399,6 @@ impl Reporter for DefaultReporter {
// If cached, the finished event above is not fired,
// so handle printing the captured logs here!
if operation.is_cached() && operation.has_output() {
self.out.write_newline()?;
self.print_operation_output(operation, item)?;
}

Expand Down
15 changes: 15 additions & 0 deletions crates/console/src/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,21 @@ impl ConsoleBuffer {
})
}

pub fn write_line_with_prefix<T: AsRef<str>>(
&self,
data: T,
prefix: &str,
) -> miette::Result<()> {
let data = data.as_ref();
let lines = data
.lines()
.map(|line| format!("{prefix}{line}"))
.collect::<Vec<_>>()
.join("\n");

self.write_line(lines)
}

pub fn write_newline(&self) -> miette::Result<()> {
self.write("\n")
}
Expand Down
1 change: 1 addition & 0 deletions crates/console/src/reporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub struct TaskReportItem {
pub attempt_current: u8,
pub attempt_total: u8,
pub hash: Option<String>,
pub output_prefix: Option<String>,
pub output_streamed: bool,
pub output_style: Option<TaskOutputStyle>,
}
Expand Down
12 changes: 6 additions & 6 deletions crates/process/src/async_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,10 @@ impl<'cmd> AsyncCommand<'cmd> {
let mut captured_lines = vec![];

while let Ok(Some(line)) = lines.next_line().await {
let _ = if stderr_prefix.is_empty() {
stderr_stream.write_line(&line)
let _ = if let Some(prefix) = &*stderr_prefix {
stderr_stream.write_line_with_prefix(&line, prefix)
} else {
stderr_stream.write_line(format!("{stderr_prefix}{line}"))
stderr_stream.write_line(&line)
};

captured_lines.push(line);
Expand All @@ -178,10 +178,10 @@ impl<'cmd> AsyncCommand<'cmd> {
let mut captured_lines = vec![];

while let Ok(Some(line)) = lines.next_line().await {
let _ = if stdout_prefix.is_empty() {
stdout_stream.write_line(&line)
let _ = if let Some(prefix) = &*stdout_prefix {
stdout_stream.write_line_with_prefix(&line, prefix)
} else {
stdout_stream.write_line(format!("{stdout_prefix}{line}"))
stdout_stream.write_line(&line)
};

captured_lines.push(line);
Expand Down
21 changes: 3 additions & 18 deletions crates/process/src/command.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{async_command::AsyncCommand, command_inspector::CommandInspector, shell::Shell};
use moon_common::{color, is_test_env};
use moon_common::color;
use moon_console::Console;
use rustc_hash::FxHashMap;
use std::{
Expand Down Expand Up @@ -187,23 +187,8 @@ impl Command {
self
}

pub fn set_prefix(&mut self, prefix: &str, width: Option<usize>) -> &mut Command {
let label = if let Some(width) = width {
format!("{: >width$}", prefix, width = width)
} else {
prefix.to_owned()
};

if is_test_env() {
self.prefix = Some(format!("{label} | "));
} else {
self.prefix = Some(format!(
"{} {} ",
color::log_target(label),
color::muted("|")
));
}

pub fn set_prefix(&mut self, prefix: &str) -> &mut Command {
self.prefix = Some(prefix.to_owned());
self
}

Expand Down
4 changes: 2 additions & 2 deletions crates/process/src/command_inspector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,8 @@ impl<'cmd> CommandInspector<'cmd> {
.get_or_init(|| CommandLine::new(self.command))
}

pub fn get_prefix(&self) -> String {
self.command.prefix.clone().unwrap_or_default()
pub fn get_prefix(&self) -> Option<String> {
self.command.prefix.clone()
}

pub fn get_workspace_root(&self) -> PathBuf {
Expand Down
18 changes: 17 additions & 1 deletion crates/target/src/target.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::target_error::TargetError;
use crate::target_scope::TargetScope;
use moon_common::{Id, ID_CHARS};
use moon_common::{color, Id, ID_CHARS};
use once_cell::sync::Lazy;
use regex::Regex;
use schematic::{Schema, SchemaBuilder, Schematic};
Expand Down Expand Up @@ -112,6 +112,22 @@ impl Target {
&self.id
}

pub fn to_prefix(&self, width: Option<usize>) -> String {
let prefix = self.as_str();

let label = if let Some(width) = width {
format!("{: >width$}", prefix, width = width)
} else {
prefix.to_owned()
};

if color::no_color() {
format!("{label} | ")
} else {
format!("{} {} ", color::log_target(label), color::muted("|"))
}
}

pub fn is_all_task(&self, task_id: &str) -> bool {
if matches!(&self.scope, TargetScope::All) {
return if let Some(id) = task_id.strip_prefix(':') {
Expand Down
39 changes: 13 additions & 26 deletions crates/task-runner/src/command_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ fn is_ci_env() -> bool {
pub struct CommandExecuteResult {
pub attempts: OperationList,
pub error: Option<miette::Report>,
pub report_item: TaskReportItem,
pub run_state: TargetState,
}

Expand Down Expand Up @@ -74,14 +73,12 @@ impl<'task> CommandExecutor<'task> {
pub async fn execute(
mut self,
context: &ActionContext,
hash: Option<&str>,
report_item: &mut TaskReportItem,
) -> miette::Result<CommandExecuteResult> {
// Prepare state for the executor, and each attempt
let mut report_item = self.prepate_state(context);
let mut run_state = TargetState::Failed;

// Hash is empty if cache is disabled
report_item.hash = hash.map(|h| h.to_string());
self.prepate_state(context, report_item);

// For long-running process, log a message on an interval to indicate it's still running
self.start_monitoring();
Expand Down Expand Up @@ -148,7 +145,7 @@ impl<'task> CommandExecutor<'task> {
"Task was successful, proceeding to next step",
);

run_state = TargetState::from_hash(hash);
run_state = TargetState::from_hash(report_item.hash.as_deref());
break None;
}
// Unsuccessful execution (maybe flaky), attempt again
Expand Down Expand Up @@ -201,7 +198,6 @@ impl<'task> CommandExecutor<'task> {
Ok(CommandExecuteResult {
attempts: self.attempts.take(),
error: execution_error,
report_item,
run_state,
})
}
Expand Down Expand Up @@ -232,8 +228,9 @@ impl<'task> CommandExecutor<'task> {
}
}

fn prepate_state(&mut self, context: &ActionContext) -> TaskReportItem {
fn prepate_state(&mut self, context: &ActionContext, report_item: &mut TaskReportItem) {
let is_primary = context.is_primary_target(&self.task.target);
let mut output_prefix = None;

// When a task is configured as local (no caching), or the interactive flag is passed,
// we don't "capture" stdout/stderr (which breaks stdin) and let it stream natively.
Expand All @@ -251,27 +248,17 @@ impl<'task> CommandExecutor<'task> {

// Transitive targets may run concurrently, so differentiate them with a prefix.
if !is_primary || is_ci_env() || context.primary_targets.len() > 1 {
let prefix_max_width = if context.primary_targets.len() > 1 {
context
.primary_targets
.iter()
.map(|target| target.id.len())
.max()
} else {
None
};
let prefix = context.get_target_prefix(&self.task.target);

self.command
.set_prefix(&self.task.target.id, prefix_max_width);
}
self.command.set_prefix(&prefix);

TaskReportItem {
attempt_current: self.attempt_index,
attempt_total: self.attempt_total,
hash: None,
output_streamed: self.stream,
output_style: self.task.options.output_style,
output_prefix = Some(prefix);
}

report_item.attempt_current = self.attempt_index;
report_item.attempt_total = self.attempt_total;
report_item.output_prefix = output_prefix;
report_item.output_streamed = self.stream;
}

fn get_command_line(&self, context: &ActionContext) -> String {
Expand Down
Loading

0 comments on commit d5801ef

Please sign in to comment.