From 67f0994b532b8740f14b5b738af7b7b2a915b1d6 Mon Sep 17 00:00:00 2001 From: Ian MacLeod Date: Sat, 31 Oct 2026 00:03:11 +0000 Subject: [PATCH] Tighten Bazel CI snapshot handling and OTLP test cleanup (#58459) ## Why Failed snapshot assertions should print their diffs without leaving `.snap.new` files after a passing retry. The OTLP loopback collector should stop once exporter shutdown completes instead of waiting for its deadline. ## What changed - Set `INSTA_UPDATE=no` for Bazel CI tests, including the local configuration used when no BuildBuddy API key is available. - Initialize the OTLP provider before starting the collector's deadline in the Tokio bounded-shutdown test. - Signal the collector to stop after shutdown and join its thread before propagating any shutdown error. GitOrigin-RevId: 55b050822ec923894c9edbc8f039f35f693a4b7e --- .bazelrc | 2 + .github/scripts/run-bazel-ci.sh | 3 ++ .../otel/tests/suite/otlp_http_loopback.rs | 51 +++++++++++-------- 3 files changed, 34 insertions(+), 22 deletions(-) diff --git a/.bazelrc b/.bazelrc index 14b941c263..b62d2d0fcc 100644 --- a/.bazelrc +++ b/.bazelrc @@ -109,6 +109,8 @@ common:remote --jobs=800 common:ci --remote_download_minimal common:ci --keep_going common:ci --verbose_failures +# A failed snapshot assertion should print its diff without leaving a .snap.new after a passing retry. +test:ci --test_env=INSTA_UPDATE=no common:ci --build_metadata=REPO_URL=https://github.com/openai/codex.git common:ci --build_metadata=ROLE=CI common:ci --build_metadata=VISIBILITY=PUBLIC diff --git a/.github/scripts/run-bazel-ci.sh b/.github/scripts/run-bazel-ci.sh index 89f937a998..07fa1ffe7b 100755 --- a/.github/scripts/run-bazel-ci.sh +++ b/.github/scripts/run-bazel-ci.sh @@ -388,6 +388,9 @@ if [[ -n "${BUILDBUDDY_API_KEY:-}" ]]; then bazel_run_args+=("--config=${ci_config}") else echo "BuildBuddy API key is not available; using local Bazel configuration." + if [[ "${bazel_args[0]}" == "test" ]]; then + bazel_run_args+=("--test_env=INSTA_UPDATE=no") + fi fi if (( ${#post_config_bazel_args[@]} > 0 )); then bazel_run_args+=("${post_config_bazel_args[@]}") diff --git a/codex-rs/otel/tests/suite/otlp_http_loopback.rs b/codex-rs/otel/tests/suite/otlp_http_loopback.rs index 1548b88c87..44b73bd898 100644 --- a/codex-rs/otel/tests/suite/otlp_http_loopback.rs +++ b/codex-rs/otel/tests/suite/otlp_http_loopback.rs @@ -640,12 +640,37 @@ async fn otlp_http_exporter_sends_traces_to_collector_with_bounded_shutdown_in_t let addr = listener.local_addr().expect("local_addr"); listener.set_nonblocking(true).expect("set_nonblocking"); + let otel = OtelProvider::try_new(&OtelSettings { + http_client_factory: HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), + environment: "test".to_string(), + service_name: "codex-cli".to_string(), + service_version: env!("CARGO_PKG_VERSION").to_string(), + codex_home: PathBuf::from("."), + exporter: OtelExporter::None, + trace_exporter: OtelExporter::OtlpHttp { + endpoint: format!("http://{addr}/v1/traces"), + headers: HashMap::new(), + protocol: OtelHttpProtocol::Json, + tls: None, + }, + metrics_exporter: OtelExporter::None, + runtime_metrics: false, + span_attributes: HashMap::new(), + tracestate: BTreeMap::new(), + })? + .expect("otel provider"); let (tx, rx) = mpsc::channel::>(); + let (stop_tx, stop_rx) = mpsc::channel(); let server = thread::spawn(move || { let mut captured = Vec::new(); let deadline = Instant::now() + Duration::from_secs(3); while Instant::now() < deadline { + match stop_rx.try_recv() { + Ok(()) | Err(mpsc::TryRecvError::Disconnected) => break, + Err(mpsc::TryRecvError::Empty) => {} + } + match listener.accept() { Ok((mut stream, _)) => { let result = read_http_request(&mut stream); @@ -667,26 +692,6 @@ async fn otlp_http_exporter_sends_traces_to_collector_with_bounded_shutdown_in_t let _ = tx.send(captured); }); - - let otel = OtelProvider::try_new(&OtelSettings { - http_client_factory: HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), - environment: "test".to_string(), - service_name: "codex-cli".to_string(), - service_version: env!("CARGO_PKG_VERSION").to_string(), - codex_home: PathBuf::from("."), - exporter: OtelExporter::None, - trace_exporter: OtelExporter::OtlpHttp { - endpoint: format!("http://{addr}/v1/traces"), - headers: HashMap::new(), - protocol: OtelHttpProtocol::Json, - tls: None, - }, - metrics_exporter: OtelExporter::None, - runtime_metrics: false, - span_attributes: HashMap::new(), - tracestate: BTreeMap::new(), - })? - .expect("otel provider"); let tracing_layer = otel.tracing_layer().expect("tracing layer"); let subscriber = tracing_subscriber::registry().with(tracing_layer); @@ -701,12 +706,14 @@ async fn otlp_http_exporter_sends_traces_to_collector_with_bounded_shutdown_in_t let _guard = span.enter(); tracing::info!("trace loopback event from tokio runtime"); }); - tokio::task::block_in_place(|| { + let shutdown_result = tokio::task::block_in_place(|| { tokio::runtime::Handle::current() .block_on(otel.shutdown_with_timeout(Duration::from_secs(/*secs*/ 2))) - })?; + }); + let _ = stop_tx.send(()); server.join().expect("server join"); + shutdown_result?; let captured = rx.recv_timeout(Duration::from_secs(1)).expect("captured"); let request = captured