diff --git a/.github/actions/build_cli_linux_windows_target/action.yaml b/.github/actions/build_cli_linux_windows_target/action.yaml index 32bbfd9d..aa1a6f6c 100644 --- a/.github/actions/build_cli_linux_windows_target/action.yaml +++ b/.github/actions/build_cli_linux_windows_target/action.yaml @@ -20,6 +20,14 @@ runs: sudo apt-get update sudo apt-get install -y musl-tools + # glibc, not musl: cross has no musl image for these, and static glibc breaks NSS DNS. + - name: Install cross binutils (glibc targets) + shell: bash + if: ${{ endsWith(inputs.target, '-unknown-linux-gnu') }} + run: | + sudo apt-get update + sudo apt-get install -y "binutils-$(echo '${{ inputs.target }}' | sed 's/-unknown//')" + - name: Build CLI ${{ inputs.target }} target uses: ./.github/actions/build_cli_target with: @@ -33,16 +41,21 @@ runs: if [[ "${{ inputs.target }}" == *"windows-gnu"* ]]; then # Windows binaries use .exe extension x86_64-w64-mingw32-strip target/${{ inputs.target }}/${{ inputs.profile }}/trunk-analytics-cli.exe + elif [[ "${{ inputs.target }}" == *"-unknown-linux-gnu" ]]; then + "$(echo '${{ inputs.target }}' | sed 's/-unknown//')-strip" -s target/${{ inputs.target }}/${{ inputs.profile }}/trunk-analytics-cli else strip -s target/${{ inputs.target }}/${{ inputs.profile }}/trunk-analytics-cli fi + # UPX cannot pack s390x, and no packed ppc64le binary has been run. - name: Install upx shell: bash + if: ${{ !endsWith(inputs.target, '-unknown-linux-gnu') }} run: | sudo apt-get install -y upx - name: Optimize CLI binary + if: ${{ !endsWith(inputs.target, '-unknown-linux-gnu') }} uses: ./.github/actions/optimize_cli_binary with: target: ${{ inputs.target }} @@ -50,7 +63,7 @@ runs: - name: Check dynamic deps (Unix) shell: bash - if: ${{ inputs.target != 'x86_64-unknown-illumos' && !contains(inputs.target, 'windows') }} + if: ${{ inputs.target != 'x86_64-unknown-illumos' && !contains(inputs.target, 'windows') && !endsWith(inputs.target, '-unknown-linux-gnu') }} run: | # Confirm that the binary is statically linked set +e @@ -61,6 +74,27 @@ runs: fi exit 0 + - name: Check dynamic deps (glibc) + shell: bash + if: ${{ endsWith(inputs.target, '-unknown-linux-gnu') }} + run: | + binary=target/${{ inputs.target }}/${{ inputs.profile }}/trunk-analytics-cli + needed=$(readelf -d "$binary" | sed -n 's/.*(NEEDED).*\[\(.*\)\]/\1/p') + echo "NEEDED: $(echo $needed)" + unexpected=$(echo "$needed" | grep -vE '^(libc|libm|libgcc_s|libpthread|libdl|librt|ld64)\.so' || true) + if [ -n "$unexpected" ]; then + echo "trunk-analytics-cli links more than glibc: $unexpected" + exit 1 + fi + # RHEL 8's glibc; Cross.toml's pinned images keep the build below it. + ceiling=2.28 + floor=$(readelf --dyn-syms -W "$binary" | grep -o 'GLIBC_[0-9.]*' | sed 's/GLIBC_//' | sort -uV | tail -1) + echo "Minimum glibc: $floor" + if [ "$(printf '%s\n' "$floor" "$ceiling" | sort -V | tail -1)" != "$ceiling" ]; then + echo "trunk-analytics-cli needs glibc $floor, newer than $ceiling" + exit 1 + fi + - name: Check dynamic deps (Windows) shell: bash if: ${{ contains(inputs.target, 'windows-gnu') }} diff --git a/.github/actions/build_cli_target/action.yaml b/.github/actions/build_cli_target/action.yaml index f0407856..4a95fe43 100644 --- a/.github/actions/build_cli_target/action.yaml +++ b/.github/actions/build_cli_target/action.yaml @@ -60,6 +60,8 @@ runs: # NOTE: DO NOT BUILD WORKSPACE OTHERWISE THE CLI MAY BE LINKED TO UNNECESSARY LIBRARIES args: "-p trunk-analytics-cli --profile=${{ inputs.profile }} --target=${{ inputs.target }} ${{ (inputs.force-sentry-dev == 'true' && '--features force-sentry-env-dev') || '' }}" use-rust-cache: false + # The latest cross release (0.2.5) ignores Cross.toml's `zig`, which ppc64le needs. + cross-version: ${{ endsWith(inputs.target, '-unknown-linux-gnu') && 'd5a5ead7397be7d49db1b12ed1a6f3583d42fb98' || '' }} - name: Build CLI ${{ inputs.target }} target shell: bash diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index be778c31..678e70b2 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -61,6 +61,14 @@ jobs: runs-on: ubuntu-24.04-arm target: aarch64-unknown-linux-musl + - os-name: linux-ppc64le + runs-on: ubuntu-latest + target: powerpc64le-unknown-linux-gnu + + - os-name: linux-s390x + runs-on: ubuntu-latest + target: s390x-unknown-linux-gnu + - os-name: x86_64-darwin runs-on: macos-latest target: x86_64-apple-darwin @@ -100,6 +108,9 @@ jobs: - name: Setup Rust & Cargo uses: ./.github/actions/setup_rust_cargo + with: + # Targets build in different images; a shared target/ runs one image's build scripts in another. + cache-key: ${{ matrix.platform.target }} - name: Build darwin target uses: ./.github/actions/build_cli_macos_target diff --git a/.github/workflows/release_cli.yml b/.github/workflows/release_cli.yml index bef23c7c..80d24030 100644 --- a/.github/workflows/release_cli.yml +++ b/.github/workflows/release_cli.yml @@ -41,6 +41,19 @@ jobs: runs-on: ubuntu-latest target: x86_64-unknown-illumos artifact-name: x86_64-unknown-illumos + skip-cli-tools: true + + - os-name: linux-ppc64le + runs-on: ubuntu-latest + target: powerpc64le-unknown-linux-gnu + artifact-name: powerpc64le-unknown-linux + skip-cli-tools: true + + - os-name: linux-s390x + runs-on: ubuntu-latest + target: s390x-unknown-linux-gnu + artifact-name: s390x-unknown-linux + skip-cli-tools: true - os-name: x86_64-darwin runs-on: macos-latest @@ -74,6 +87,9 @@ jobs: - name: Setup Rust & Cargo uses: ./.github/actions/setup_rust_cargo + with: + # Targets build in different images; a shared target/ runs one image's build scripts in another. + cache-key: ${{ matrix.platform.target }} - name: Update CLI version uses: ./.github/actions/update_version @@ -115,7 +131,7 @@ jobs: ######################################################### - name: Build CLI Tools uses: ./.github/actions/build_cli_tools - if: matrix.platform.target != 'x86_64-unknown-illumos' + if: ${{ !matrix.platform.skip-cli-tools }} with: target: ${{ matrix.platform.target }} os-name: ${{ matrix.platform.os-name }} @@ -153,7 +169,7 @@ jobs: - name: Upload Bundle CLI artifact (Unix) uses: actions/upload-artifact@v4 - if: ${{ matrix.platform.target != 'x86_64-unknown-illumos' && !contains(matrix.platform.os-name, 'windows') }} + if: ${{ !matrix.platform.skip-cli-tools && !contains(matrix.platform.os-name, 'windows') }} with: name: ${{ matrix.platform.artifact-name }}-bundle path: target/${{ matrix.platform.target }}/release-with-debug/proto-bin-to-json @@ -169,7 +185,7 @@ jobs: - name: Upload XCResult CLI artifact (Unix) uses: actions/upload-artifact@v4 - if: ${{ matrix.platform.target != 'x86_64-unknown-illumos' && !contains(matrix.platform.os-name, 'windows') }} + if: ${{ !matrix.platform.skip-cli-tools && !contains(matrix.platform.os-name, 'windows') }} with: name: ${{ matrix.platform.artifact-name }}-xcresult path: target/${{ matrix.platform.target }}/release-with-debug/xcresult-to-junit @@ -185,7 +201,7 @@ jobs: - name: Upload Codeowners CLI artifact (Unix) uses: actions/upload-artifact@v4 - if: ${{ matrix.platform.target != 'x86_64-unknown-illumos' && !contains(matrix.platform.os-name, 'windows') }} + if: ${{ !matrix.platform.skip-cli-tools && !contains(matrix.platform.os-name, 'windows') }} with: name: ${{ matrix.platform.artifact-name }}-codeowners path: target/${{ matrix.platform.target }}/release-with-debug/check-codeowners @@ -206,6 +222,17 @@ jobs: - name: windows os: windows-latest target: x86_64-pc-windows-gnu + # GitHub has no ppc64le or s390x runners, so these run under QEMU user-mode. + - name: linux-ppc64le + os: ubuntu-latest + target: powerpc64le-unknown-linux + qemu-triple: powerpc64le-linux-gnu + qemu-deb-arch: ppc64el + - name: linux-s390x + os: ubuntu-latest + target: s390x-unknown-linux + qemu-triple: s390x-linux-gnu + qemu-deb-arch: s390x needs: - build_release steps: @@ -218,6 +245,17 @@ jobs: name: ${{ matrix.type.target }} path: ./trunk-analytics-cli + - name: Set up QEMU user-mode emulation + if: ${{ matrix.type.qemu-triple }} + shell: bash + run: | + sudo apt-get update + sudo apt-get install -y qemu-user-static binfmt-support \ + libc6-${{ matrix.type.qemu-deb-arch }}-cross libgcc-s1-${{ matrix.type.qemu-deb-arch }}-cross + echo "QEMU_LD_PREFIX=/usr/${{ matrix.type.qemu-triple }}" >> "$GITHUB_ENV" + chmod +x trunk-analytics-cli/trunk-analytics-cli + QEMU_LD_PREFIX=/usr/${{ matrix.type.qemu-triple }} trunk-analytics-cli/trunk-analytics-cli --version + - name: Setup Run Smoke Tests uses: ./.github/actions/setup_run_smoke_tests diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5289cf7c..2cc6bf24 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -17,6 +17,29 @@ cargo build The CLI will be built to `target/debug/trunk-analytics-cli` +### Cross-Compiling for Other Linux Architectures + +Release targets your machine cannot build natively (`powerpc64le-unknown-linux-gnu`, `s390x-unknown-linux-gnu`, `x86_64-unknown-illumos`) build in Docker with [cross](https://github.com/cross-rs/cross): + +```bash +cargo install cross --git https://github.com/cross-rs/cross +cross build -p trunk-analytics-cli --profile=release --target=s390x-unknown-linux-gnu +``` + +Build only `-p trunk-analytics-cli`, as CI does; building the workspace links libraries the release binary must not need. + +The ppc64le and s390x builds link glibc dynamically, and `Cross.toml` keeps them at glibc 2.28 or older so they run on RHEL 8 (zig for ppc64le, which the cross 0.2.5 release ignores; an old image for s390x). CI fails a build that needs a newer glibc. Use a fresh `CARGO_TARGET_DIR` when switching a target's image: build scripts compiled in one image may not run in another. + +To try a binary, run it under the QEMU that ships in its cross image: + +```bash +docker run --rm -v "$PWD:/repo" -w /repo ghcr.io/cross-rs/s390x-unknown-linux-gnu:0.2.5 \ + /linux-runner s390x target/s390x-unknown-linux-gnu/release/trunk-analytics-cli \ + validate --junit-paths cli/test_fixtures/junit0_fail.xml +``` + +s390x is big-endian, so it is the target most likely to show a byte-order bug. + ### Package-Specific Build Instructions For detailed build instructions for each supported package, see their respective README files: diff --git a/Cross.toml b/Cross.toml index 09809e19..16afefd2 100644 --- a/Cross.toml +++ b/Cross.toml @@ -2,3 +2,12 @@ passthrough = [ "RANLIB=/usr/local/x86_64-unknown-illumos/bin/x86_64-unknown-illumos-ranlib", ] + +# cross's main images link against glibc 2.39, newer than RHEL 8/9 and SLES 15. ppc64le links +# through zig, because a binary linked in the old 0.2.5 image crashes on glibc 2.34 and newer; +# zig cannot link s390x, whose 0.2.5-image binary runs on every glibc. +[target.powerpc64le-unknown-linux-gnu] +zig = "2.28" + +[target.s390x-unknown-linux-gnu] +image = "ghcr.io/cross-rs/s390x-unknown-linux-gnu:0.2.5" diff --git a/cli/src/summary_output/github.rs b/cli/src/summary_output/github.rs index eeee6a16..2aff36b8 100644 --- a/cli/src/summary_output/github.rs +++ b/cli/src/summary_output/github.rs @@ -10,8 +10,9 @@ use super::{ ErrorSummary, FailedTest, Outcome, Quarantining, UploadReport, json::Summary, text::{ - FAILED_WITHOUT_RUNNING_TESTS, FAILURE_MESSAGE_MAX_LINES, NO_TEST_FAILURES, NO_TESTS_FOUND, - QUARANTINE_LOOKUP_FAILED, SOME_TESTS_MAY_NOT_HAVE_RUN, UNAUTHORIZED_HINT, + BAZEL_BEP_HINT, FAILED_WITHOUT_RUNNING_TESTS, FAILURE_MESSAGE_MAX_LINES, NO_TEST_FAILURES, + NO_TESTS_FOUND, QUARANTINE_LOOKUP_FAILED, SOME_TESTS_MAY_NOT_HAVE_RUN, UNAUTHORIZED_HINT, + old_timestamps_message, }, }; use crate::{ @@ -74,6 +75,12 @@ fn upload_annotations(report: &UploadReport) -> Vec { } annotations.push(Annotation::warning(error_message(error))); } + if report.files_with_old_timestamps > 0 { + annotations.push(Annotation::warning(format!( + "{}, but the test counts include them.\n{BAZEL_BEP_HINT}", + old_timestamps_message(report.files_with_old_timestamps) + ))); + } let exit_code = report.cli_exit_code(); match report.outcome { diff --git a/cli/src/summary_output/text.rs b/cli/src/summary_output/text.rs index 6763f439..1ef0571f 100644 --- a/cli/src/summary_output/text.rs +++ b/cli/src/summary_output/text.rs @@ -1,5 +1,6 @@ use std::path::{Path, PathBuf}; +use context::junit::validator::TIMESTAMP_OLD_HOURS; use pluralizer::pluralize; use superconsole::{ Hyperlink, Line, Lines, Span, SuperConsole, @@ -81,6 +82,16 @@ pub(crate) const SOME_TESTS_MAY_NOT_HAVE_RUN: &str = "This may indicate that som pub(crate) const QUARANTINE_LOOKUP_FAILED: &str = "We were unable to determine the quarantine status for tests. Any failing tests will be reported as failures"; pub(crate) const FAILURE_MESSAGE_MAX_LINES: usize = 20; pub(crate) const UNAUTHORIZED_HINT: &str = "Hint: You can find it under the settings page at"; +pub(crate) const BAZEL_BEP_HINT: &str = "If Bazel restored these files from its cache, upload with --bazel-bep-path to skip cached results: https://docs.trunk.io/flaky-tests/get-started/frameworks/bazel"; + +/// Shown whatever `--validation-report` says, since Trunk's ingestion drops these files' results. +pub(crate) fn old_timestamps_message(files: usize) -> String { + format!( + "{} {} test results older than {TIMESTAMP_OLD_HOURS} hours. Trunk will drop these results", + pluralize("file", files as isize, true), + if files == 1 { "has" } else { "have" }, + ) +} pub fn render_error(error: &ErrorSummary) -> anyhow::Result> { let mut lines = Vec::new(); @@ -209,6 +220,12 @@ pub fn render_upload(report: &UploadReport, options: &TextOptions) -> anyhow::Re .attribute(Attribute::Bold), )?])); } + if report.files_with_old_timestamps > 0 { + output.extend(render_old_timestamps(format!( + "{}, but the test counts above include them.", + old_timestamps_message(report.files_with_old_timestamps) + ))?); + } output.push(Line::default()); if quarantined > 0 { @@ -310,6 +327,17 @@ pub fn render_upload(report: &UploadReport, options: &TextOptions) -> anyhow::Re Ok(output) } +fn render_old_timestamps(message: String) -> anyhow::Result> { + Ok(vec![ + Line::from_iter([Span::new_styled_lossy( + style(format!(" ⚠️ {message}")) + .with(Color::Yellow) + .attribute(Attribute::Bold), + )]), + Line::from_iter([Span::new_unstyled(format!(" {BAZEL_BEP_HINT}"))?]), + ]) +} + fn render_groups(groups: &[FailureGroup], options: &TextOptions) -> anyhow::Result { let mut output = Vec::new(); for group in groups { @@ -663,6 +691,7 @@ pub fn render_validate(report: &ValidateReport) -> anyhow::Result> { files_with_errors, warnings, errors, + files_with_old_timestamps, } = report.counts; output.push(Line::from_iter([ Span::new_styled(style(format!("{}", valid_files)).with(Color::Green))?, @@ -694,6 +723,13 @@ pub fn render_validate(report: &ValidateReport) -> anyhow::Result> { Span::new_styled(style(format!("{}", errors)).with(Color::Red))?, Span::new_unstyled(format!(" {}", pluralize("error", errors as isize, false)))?, ])); + if files_with_old_timestamps > 0 { + output.push(Line::default()); + output.extend(render_old_timestamps(format!( + "{} when they're uploaded.", + old_timestamps_message(files_with_old_timestamps) + ))?); + } Ok(output) } diff --git a/cli/src/summary_output/upload.rs b/cli/src/summary_output/upload.rs index 1569e31f..8e3f55df 100644 --- a/cli/src/summary_output/upload.rs +++ b/cli/src/summary_output/upload.rs @@ -35,6 +35,10 @@ pub struct UploadReport { pub upload_url: Option, /// One entry per test report file. pub validations: Vec, + /// Files whose results Trunk will drop for having old timestamps. `counts` includes their + /// tests. + #[serde(default)] + pub files_with_old_timestamps: usize, pub counts: TestCounts, pub quarantining: Quarantining, /// Failed tests that were quarantined, grouped like `failures`. @@ -114,6 +118,9 @@ pub struct FileValidation { pub test_suite_count: Option, /// Null when the file couldn't be parsed. pub test_case_count: Option, + /// The file has a test older than 24 hours, so Trunk will drop all of its results. + #[serde(default)] + pub old_timestamps: bool, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] @@ -179,6 +186,7 @@ impl UploadReport { UploadCommand::RSpec => None, }; let bundle_upload_id = &result.meta.base_props.bundle_upload_id; + let validations = file_validations(&result.validations); Self { command, @@ -186,7 +194,8 @@ impl UploadReport { dry_run: result.dry_run, bundle_upload_id: Some(bundle_upload_id.clone()).filter(|id| !id.is_empty()), upload_url: upload_url(result), - validations: file_validations(&result.validations), + files_with_old_timestamps: files_with_old_timestamps(&validations), + validations, counts, quarantining: quarantine_context.quarantining(result.disable_quarantining), quarantined: group(quarantined, |test| failed_test(result, test, true)), @@ -228,6 +237,13 @@ impl TestCounts { } } +pub(super) fn files_with_old_timestamps(validations: &[FileValidation]) -> usize { + validations + .iter() + .filter(|validation| validation.old_timestamps) + .count() +} + impl FileValidation { pub fn has_issues(&self) -> bool { self.parse_error.is_some() || !self.errors.is_empty() || !self.warnings.is_empty() @@ -336,6 +352,7 @@ fn file_validations(validations: &JunitReportValidations) -> Vec warnings: Vec::new(), test_suite_count: None, test_case_count: None, + old_timestamps: false, }, Ok(validation) => { let issues = validation.all_issues_flat(); @@ -353,6 +370,7 @@ fn file_validations(validations: &JunitReportValidations) -> Vec warnings: messages(JunitValidationLevel::SubOptimal), test_suite_count: Some(validation.test_suites().len()), test_case_count: Some(validation.test_cases().len()), + old_timestamps: validation.has_old_timestamps(), } } }) diff --git a/cli/src/summary_output/validate.rs b/cli/src/summary_output/validate.rs index 35bbc1d2..2a929e5e 100644 --- a/cli/src/summary_output/validate.rs +++ b/cli/src/summary_output/validate.rs @@ -4,7 +4,7 @@ use context::junit::validator::JunitReportValidationFlatIssue; use schemars::JsonSchema; use serde::Serialize; -use super::{FileValidation, Notice, ParseError}; +use super::{FileValidation, Notice, ParseError, upload::files_with_old_timestamps}; use crate::{ exit_code::ExitCode, validate_command::{ParseIssues, TestIssues, ValidateRunResult}, @@ -49,6 +49,8 @@ pub struct ValidationCounts { pub files_with_errors: usize, pub warnings: usize, pub errors: usize, + /// Files whose results Trunk will drop for having old timestamps. + pub files_with_old_timestamps: usize, } impl ValidateReport { @@ -132,6 +134,7 @@ impl ValidationCounts { files_with_errors: files_with_errors.len(), warnings, errors, + files_with_old_timestamps: files_with_old_timestamps(validations), } } } @@ -145,6 +148,7 @@ fn parse_validation(issues: &ParseIssues) -> FileValidation { warnings: issues.warnings.iter().map(ToString::to_string).collect(), test_suite_count: None, test_case_count: None, + old_timestamps: false, } } @@ -162,5 +166,6 @@ fn test_validation(issues: &TestIssues) -> FileValidation { warnings: messages(&issues.warnings), test_suite_count: Some(issues.num_test_suites), test_case_count: Some(issues.num_test_cases), + old_timestamps: issues.old_timestamps, } } diff --git a/cli/src/validate_command.rs b/cli/src/validate_command.rs index 66204f6c..fb7af472 100644 --- a/cli/src/validate_command.rs +++ b/cli/src/validate_command.rs @@ -102,6 +102,7 @@ pub struct TestIssues { pub(crate) num_test_cases: usize, pub(crate) errors: Vec, pub(crate) warnings: Vec, + pub(crate) old_timestamps: bool, } #[derive(Debug)] @@ -448,6 +449,7 @@ fn gen_test_issues(report_validations: &JunitFileToValidation) -> Vec JunitReportValidations { + JunitReportValidations::new( + (0..num_files) + .map(|n| { + ( + format!("test-results/{n:02}.xml"), + validation(vec![ + report_warning(JunitReportValidationIssueSubOptimal::MissingTimestamps), + report_warning(JunitReportValidationIssueSubOptimal::OldTimestamps), + ]), + ) + }) + .collect(), + ) +} + +#[test] +fn counts_every_file_with_old_timestamps_past_the_file_limit() { + let mut result = upload_result(1344); + result.validations = old_timestamp_validations(112); + + let summary = upload_summary(&result); + assert_eq!( + summary[summary.len() - 8..].to_vec(), + [ + "…and 104 more", + "", + "πŸ“š Test Report", + " Total: 1344 Pass: 1344 Fail: 0 Quarantined: 0 Pass Ratio: 100.0%", + " ⚠️ 112 files have test results older than 24 hours. Trunk will drop these results, but the test counts above include them.", + " If Bazel restored these files from its cache, upload with --bazel-bep-path to skip cached results: https://docs.trunk.io/flaky-tests/get-started/frameworks/bazel", + "", + "πŸŽ‰ No test failures found!", + ] + ); +} + +#[test] +fn writes_the_files_with_old_timestamps_as_json() { + let mut result = upload_result(4); + result.validations = JunitReportValidations::new(BTreeMap::from([ + ( + String::from("test-results/new.xml"), + validation(vec![report_warning( + JunitReportValidationIssueSubOptimal::StaleTimestamps, + )]), + ), + ( + String::from("test-results/old.xml"), + validation(vec![report_warning( + JunitReportValidationIssueSubOptimal::OldTimestamps, + )]), + ), + ])); + + let json = upload_json(&result); + assert_eq!(json["filesWithOldTimestamps"], 1); + assert_eq!( + json["validations"] + .as_array() + .unwrap() + .iter() + .map(|validation| (&validation["file"], &validation["oldTimestamps"])) + .collect::>(), + [ + ( + &serde_json::json!("test-results/new.xml"), + &serde_json::json!(false) + ), + ( + &serde_json::json!("test-results/old.xml"), + &serde_json::json!(true) + ), + ] + ); +} + fn upload_json(result: &UploadRunResult) -> serde_json::Value { let report = UploadReport::new(result, UploadCommand::Upload); serde_json::from_str(&Report::Upload(&report).to_json().unwrap()).unwrap() @@ -996,6 +1081,7 @@ fn writes_the_report_as_json() { "warnings": ["test case or parent has no time duration"], "testSuiteCount": 0, "testCaseCount": 0, + "oldTimestamps": false, }, { "file": "test-results/b.xml", @@ -1007,8 +1093,10 @@ fn writes_the_report_as_json() { "warnings": [], "testSuiteCount": null, "testCaseCount": null, + "oldTimestamps": false, }, ], + "filesWithOldTimestamps": 0, "counts": { "total": 10, "passed": 7, diff --git a/cli/tests/summary/validate.rs b/cli/tests/summary/validate.rs index aab17ee3..0fbba1fd 100644 --- a/cli/tests/summary/validate.rs +++ b/cli/tests/summary/validate.rs @@ -121,3 +121,38 @@ async fn prints_bep_parse_errors_as_notices_without_a_codeowners_file() { ] ); } + +/// One test that ran two days ago: old enough for Trunk to drop, too recent for the validator +/// to discard as a defaulted timestamp. +fn old_timestamp_junit_xml() -> String { + let timestamp = (chrono::Utc::now() - chrono::TimeDelta::days(2)).format("%Y-%m-%dT%H:%M:%SZ"); + format!( + r#" + + + + + +"# + ) +} + +#[tokio::test] +async fn says_that_results_with_old_timestamps_will_be_dropped() { + let temp_dir = tempfile::tempdir().unwrap(); + let path = temp_dir.path().join("old.xml"); + std::fs::write(&path, old_timestamp_junit_xml()).unwrap(); + let report = validate_report(&["--junit-paths", path.to_str().unwrap()]).await; + assert_eq!(report.counts.files_with_old_timestamps, 1); + let summary = unstyled(text::render_validate(&report)); + assert_eq!( + summary[summary.len() - 5..].to_vec(), + [ + "0 valid files, 1 file with warnings, and 0 files with errors, with 1 file total (a file is double counted if it has both errors and warnings)", + "1 warning, and 0 errors", + "", + " ⚠️ 1 file has test results older than 24 hours. Trunk will drop these results when they're uploaded.", + " If Bazel restored these files from its cache, upload with --bazel-bep-path to skip cached results: https://docs.trunk.io/flaky-tests/get-started/frameworks/bazel", + ] + ); +} diff --git a/cli/tests/upload.rs b/cli/tests/upload.rs index 67693004..03d8cd05 100644 --- a/cli/tests/upload.rs +++ b/cli/tests/upload.rs @@ -1619,6 +1619,65 @@ async fn upload_writes_summary_output_file() { } } +/// One test that ran two days ago: old enough for Trunk to drop, too recent for the validator +/// to discard as a defaulted timestamp. +fn old_timestamp_junit_xml() -> String { + let timestamp = (chrono::Utc::now() - chrono::TimeDelta::days(2)).format("%Y-%m-%dT%H:%M:%SZ"); + format!( + r#" + + + + + +"# + ) +} + +#[tokio::test(flavor = "multi_thread")] +async fn upload_says_how_many_files_have_old_timestamps_past_the_file_limit() { + let temp_dir = tempdir().unwrap(); + generate_mock_git_repo(&temp_dir); + for n in 0..10 { + fs::write( + temp_dir.path().join(format!("old-{n}.xml")), + old_timestamp_junit_xml(), + ) + .unwrap(); + } + let summary_path = temp_dir.path().join("summary.json"); + + let state = MockServerBuilder::new().spawn_mock_server().await; + let assert = CommandBuilder::upload(temp_dir.path(), state.host.clone()) + .junit_paths("old-*.xml") + .command() + .arg("--summary-output-file") + .arg(&summary_path) + .assert() + .success() + .stdout(predicate::str::contains("…and 2 more")); + + // Listed once, though 2 of the 10 files are cut from the per-file warnings. + let stdout = String::from_utf8_lossy(&assert.get_output().stdout).into_owned(); + assert_eq!( + stdout + .matches( + "10 files have test results older than 24 hours. Trunk will drop these results, but the test counts above include them." + ) + .count(), + 1 + ); + assert!(stdout.contains("upload with --bazel-bep-path to skip cached results")); + + let requests = state.requests.lock().unwrap().clone(); + assert_eq!(requests.len(), 3); + assert_matches!(requests[1], RequestPayload::S3Upload(_)); + + let summary = read_summary_output_file(&summary_path); + assert_eq!(summary["filesWithOldTimestamps"], 10); + assert_eq!(summary["counts"]["total"], 10); +} + #[tokio::test(flavor = "multi_thread")] async fn upload_writes_a_dry_run_to_summary_output_file() { let temp_dir = tempdir().unwrap(); diff --git a/context/src/junit/validator.rs b/context/src/junit/validator.rs index f920e4be..97a4ac30 100644 --- a/context/src/junit/validator.rs +++ b/context/src/junit/validator.rs @@ -358,6 +358,18 @@ impl JunitReportValidation { .collect() } + /// Trunk drops a report with old timestamps when it ingests the upload. + pub fn has_old_timestamps(&self) -> bool { + self.all_issues.iter().any(|issue| { + matches!( + issue, + JunitValidationIssueType::Report(JunitValidationIssue::SubOptimal( + JunitReportValidationIssueSubOptimal::OldTimestamps + )) + ) + }) + } + pub fn test_suites(&self) -> &[JunitTestSuiteValidation] { &self.test_suites }