skills/rtk-ai__rtk
1
0
Fork 0
mirror of https://github.com/rtk-ai/rtk.git synced 2026-09-19 07:33:17 +08:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3430 from ousamabenyounes/fix/benchmark-negative-curl-cargo

Browse Source 
fix(benchmark): avoid negative curl/cargo cases that fail the benchmark job
This commit is contained in:
Nicolas Le Cam
2026-08-17 03:03:41 +02:00
committed by GitHub
parent 32c83cc60a d003920601
commit ba7a9ce0d9
2 changed files with 205 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files
scripts/benchmark.sh
+87 -10
View File
@@ -342,21 +342,98 @@ section "wc"
bench "wc" "wc Cargo.toml src/main.rs" "$RTK wc Cargo.toml src/main.rs"
# ===================
# curl
# curl / wget — fully offline. mockhttp.org was both a network dependency and
# non-deterministic (its /json output is random), which made the benchmark flaky.
# Serve fixed fixtures locally instead. The JSON is pretty-printed on purpose so
# `rtk` actually exercises JSON minification (a pre-minified body leaves nothing
# to compact).
# ===================
section "curl"
if command -v curl &> /dev/null; then
bench "curl json" "curl -s https://mockhttp.org/json/1" "$RTK curl https://mockhttp.org/json/1"
bench "curl text" "curl -s https://mockhttp.org/robots.txt" "$RTK curl https://mockhttp.org/robots.txt"
NET_FIXTURE_DIR="$(mktemp -d)"
# Server log lives outside the served directory so it is never itself served.
NET_HTTP_LOG="$(mktemp)"
NET_HTTP_PID=""
# Every step is failure-tolerant: this runs from an EXIT trap under `set -e`, so
# a single failing command (e.g. `kill` on a server that already died) would
# otherwise abort the trap and leak the fixture dir and downloaded files.
cleanup_net_fixtures() {
if [ -n "$NET_HTTP_PID" ]; then
kill "$NET_HTTP_PID" 2>/dev/null || true
fi
rm -rf "$NET_FIXTURE_DIR" "$NET_HTTP_LOG" || true
# `rtk wget <url>/data.json` saves to ./data.json (default basename); remove
# that download and any numbered duplicates from repeated runs.
rm -f data.json data.json.* 2>/dev/null || true
}
trap cleanup_net_fixtures EXIT
cat > "$NET_FIXTURE_DIR/data.json" << 'JSONEOF'
{
"message": "Hello from RTK benchmark",
"status": "success",
"code": 200,
"items": [
{ "id": 1, "name": "first" },
{ "id": 2, "name": "second" }
]
}
JSONEOF
cat > "$NET_FIXTURE_DIR/robots.txt" << 'TXTEOF'
User-agent: *
Disallow: /private/
Allow: /
Sitemap: https://example.com/sitemap.xml
TXTEOF
# Bring up a loopback HTTP server once so both curl and wget get real response
# headers (`Content-Type: application/json`) — that's what lets rtk detect JSON
# and minify it. file:// carries no headers, so it can't demonstrate that path.
#
# Port 0 asks the kernel for a free port, so a busy fixed port can never make the
# benchmark fail. `python3 -u` keeps stdout unbuffered, so the "Serving HTTP on
# 127.0.0.1 port NNNNN" line — printed only once the socket is bound and
# listening — reaches the log as soon as the server is ready.
readonly NET_HTTP_EPHEMERAL_PORT=0
readonly NET_HTTP_READY_ATTEMPTS=25
readonly NET_HTTP_READY_DELAY=0.2
NET_HTTP_URL=""
if command -v python3 &> /dev/null; then
( cd "$NET_FIXTURE_DIR" \
&& exec python3 -u -m http.server "$NET_HTTP_EPHEMERAL_PORT" --bind 127.0.0.1 ) \
> "$NET_HTTP_LOG" 2>&1 &
NET_HTTP_PID=$!
for _ in $(seq 1 "$NET_HTTP_READY_ATTEMPTS"); do
net_http_port="$(sed -n 's/.*port \([0-9][0-9]*\).*/\1/p' "$NET_HTTP_LOG" | head -1)"
if [ -n "$net_http_port" ]; then
NET_HTTP_URL="http://127.0.0.1:$net_http_port"
break
fi
sleep "$NET_HTTP_READY_DELAY"
done
fi
# ===================
# wget
# ===================
section "curl"
if command -v curl &> /dev/null; then
if [ -n "$NET_HTTP_URL" ]; then
bench "curl json" "curl -s $NET_HTTP_URL/data.json" "$RTK curl $NET_HTTP_URL/data.json"
bench "curl text" "curl -s $NET_HTTP_URL/robots.txt" "$RTK curl $NET_HTTP_URL/robots.txt"
else
# No python3 to host a local server — fall back to file:// (no headers, so
# no JSON minification, but still fully offline and deterministic).
bench "curl json" "curl -s file://$NET_FIXTURE_DIR/data.json" "$RTK curl file://$NET_FIXTURE_DIR/data.json"
bench "curl text" "curl -s file://$NET_FIXTURE_DIR/robots.txt" "$RTK curl file://$NET_FIXTURE_DIR/robots.txt"
fi
fi
# wget has no offline fallback: it rejects file:// outright ("Unsupported
# scheme"), so without the loopback server there is nothing local to fetch. Say
# so explicitly instead of letting the case disappear from the report.
if command -v wget &> /dev/null; then
section "wget"
bench "wget" "wget -qO- https://mockhttp.org/json/1" "$RTK wget https://mockhttp.org/json/1"
rm -f 1 2>/dev/null
if [ -n "$NET_HTTP_URL" ]; then
bench "wget" "wget -qO- $NET_HTTP_URL/data.json" "$RTK wget $NET_HTTP_URL/data.json"
else
echo "⏭️ wget (no local HTTP server available, skipped)"
fi
fi
# ===================
src/cmds/rust/cargo_cmd.rs
+118 -59
View File
@@ -96,13 +96,14 @@ impl BlockHandler for CargoBuildHandler {
!(line.trim().is_empty() && block.len() > 3)
}
fn format_summary(&self, exit_code: i32, _raw: &str) -> Option<String> {
fn format_summary(&self, exit_code: i32, raw: &str) -> Option<String> {
if self.error_count == 0 && self.warnings == 0 && exit_code == 0 {
return Some(cargo_build_success_line(
let summary = cargo_build_success_line(
self.compiled,
self.finished_line.as_deref(),
self.label,
));
);
return Some(crate::core::guard::never_worse(raw, &summary).to_string());
}
// The streamed path only runs for non-json build/check; error blocks are
// emitted live, so the summary carries no rendered diagnostics.
@@ -137,6 +138,65 @@ impl CargoTestHandler {
has_compile_errors: false,
}
}
/// Compacted `cargo test` summary, before the never-worse guard.
fn compute_test_summary(&self, raw: &str) -> Option<String> {
if self.summary_lines.is_empty() {
let json = extract_json_diagnostics(raw);
if self.has_compile_errors || !json.errors.is_empty() {
// Content-based (exit 0): a real compile error yields "cargo test: N
// errors"; a bare "could not compile" leaves the raw tail fallback.
let build_filtered = filter_cargo_build_labeled(raw, "test", 0);
if build_filtered.contains("cargo test:") {
return Some(format!("{}\n", build_filtered));
}
// Fallback: last 5 meaningful lines
let meaningful: Vec<&str> = raw
.lines()
.filter(|l| !l.trim().is_empty() && !l.trim_start().starts_with("Compiling"))
.collect();
let last5: Vec<&str> = meaningful.iter().rev().take(5).rev().copied().collect();
return Some(format!("{}\n", last5.join("\n")));
}
}
// No failures emitted — aggregate pass results
let mut aggregated: Option<AggregatedTestResult> = None;
let mut all_parsed = true;
for line in &self.summary_lines {
if let Some(parsed) = AggregatedTestResult::parse_line(line) {
if let Some(ref mut agg) = aggregated {
agg.merge(&parsed);
} else {
aggregated = Some(parsed);
}
} else {
all_parsed = false;
break;
}
}
if all_parsed {
if let Some(agg) = aggregated {
if agg.suites > 0 {
return Some(format!("{}\n", agg.format_compact()));
}
}
}
// Fallback: show raw summary lines
if !self.summary_lines.is_empty() {
let mut s = String::new();
for line in &self.summary_lines {
s.push_str(line);
s.push('\n');
}
return Some(s);
}
None
}
}
impl BlockHandler for CargoTestHandler {
@@ -195,61 +255,12 @@ impl BlockHandler for CargoTestHandler {
}
fn format_summary(&self, _exit_code: i32, raw: &str) -> Option<String> {
if self.summary_lines.is_empty() {
let json = extract_json_diagnostics(raw);
if self.has_compile_errors || !json.errors.is_empty() {
// Content-based (exit 0): a real compile error yields "cargo test: N
// errors"; a bare "could not compile" leaves the raw tail fallback.
let build_filtered = filter_cargo_build_labeled(raw, "test", 0);
if build_filtered.contains("cargo test:") {
return Some(format!("{}\n", build_filtered));
}
// Fallback: last 5 meaningful lines
let meaningful: Vec<&str> = raw
.lines()
.filter(|l| !l.trim().is_empty() && !l.trim_start().starts_with("Compiling"))
.collect();
let last5: Vec<&str> = meaningful.iter().rev().take(5).rev().copied().collect();
return Some(format!("{}\n", last5.join("\n")));
}
}
// No failures emitted — aggregate pass results
let mut aggregated: Option<AggregatedTestResult> = None;
let mut all_parsed = true;
for line in &self.summary_lines {
if let Some(parsed) = AggregatedTestResult::parse_line(line) {
if let Some(ref mut agg) = aggregated {
agg.merge(&parsed);
} else {
aggregated = Some(parsed);
}
} else {
all_parsed = false;
break;
}
}
if all_parsed {
if let Some(agg) = aggregated {
if agg.suites > 0 {
return Some(format!("{}\n", agg.format_compact()));
}
}
}
// Fallback: show raw summary lines
if !self.summary_lines.is_empty() {
let mut s = String::new();
for line in &self.summary_lines {
s.push_str(line);
s.push('\n');
}
return Some(s);
}
None
// Same never-worse guard as CargoBuildHandler (#3430 review): if the
// compacted summary ends up larger than the raw output (e.g. a tiny
// `cargo test` run), keep the raw output instead of "compacting" it
// into something bigger.
let summary = self.compute_test_summary(raw)?;
Some(crate::core::guard::never_worse(raw, &summary).to_string())
}
}
@@ -988,7 +999,8 @@ fn filter_cargo_build_labeled(output: &str, label: &'static str, exit_code: i32)
let (errors, warnings) = merge_diag_counts(handler.error_count, handler.warnings, &json);
if errors == 0 && warnings == 0 && exit_code == 0 {
return cargo_build_success_line(handler.compiled, handler.finished_line.as_deref(), label);
let summary = cargo_build_success_line(handler.compiled, handler.finished_line.as_deref(), label);
return crate::core::guard::never_worse(output, &summary).to_string();
}
let mut result =
@@ -1437,6 +1449,9 @@ pub fn run_passthrough(args: &[OsString], verbose: u8) -> Result<i32> {
#[cfg(test)]
mod tests {
const TINY_BUILD_SUCCESS_OUTPUT: &str =
" Finished dev [unoptimized + debuginfo] target(s) in 0.01s\n";
use super::*;
use crate::core::args_utils::restore_double_dash_with_raw;
@@ -1557,6 +1572,42 @@ mod tests {
assert!(result.contains("3 crates compiled"));
}
#[test]
fn test_filter_cargo_build_success_uses_raw_when_summary_is_larger() {
let output = TINY_BUILD_SUCCESS_OUTPUT;
let result = filter_cargo_build(output);
assert_eq!(result, output);
}
#[test]
fn test_cargo_test_summary_uses_raw_when_summary_is_larger() {
// A one-line compile failure is already minimal: prefixing it with the
// "cargo test: N errors, ..." header emits more tokens than the raw
// output, so the never-worse guard must keep the raw output.
const CARGO_COMPILE_FAILURE_EXIT_CODE: i32 = 101;
let raw = "error[E0433]: failed to resolve: use of undeclared type `Foo`\n";
let mut handler = CargoTestHandler::new();
for line in raw.lines() {
handler.should_skip(line);
}
let unguarded = handler
.compute_test_summary(raw)
.expect("compile failure yields a summary");
assert!(
unguarded.len() > raw.len(),
"expected the compacted summary to be larger than the raw output, got {:?}",
unguarded
);
assert_eq!(
handler
.format_summary(CARGO_COMPILE_FAILURE_EXIT_CODE, raw)
.as_deref(),
Some(raw)
);
}
#[test]
fn test_filter_cargo_build_errors() {
let output = r#" Compiling rtk v0.5.0
@@ -2314,6 +2365,14 @@ error: test run failed
assert!(!result.contains("Compiling"), "got: {}", result);
}
#[test]
fn test_cargo_build_stream_success_uses_raw_when_summary_is_larger() {
let input = TINY_BUILD_SUCCESS_OUTPUT;
let mut f = BlockStreamFilter::new(CargoBuildHandler::with_label("build"));
let result = run_block_filter(&mut f, input, 0);
assert_eq!(result, input);
}
#[test]
fn test_cargo_build_stream_json_success() {
let input = concat!(