Skip to content

Add analytics-cli upload troubleshooting and update install path#170

Draft
samgutentag wants to merge 1 commit into
mainfrom
sam-gutentag/analytics-cli-troubleshooting
Draft

Add analytics-cli upload troubleshooting and update install path#170
samgutentag wants to merge 1 commit into
mainfrom
sam-gutentag/analytics-cli-troubleshooting

Conversation

@samgutentag
Copy link
Copy Markdown
Member

Reopens the work from #54, which was merged then reverted on 2026-06-01 (it was not ready to merge). Content is unchanged from the original branch; needs content verification against source before re-merging. Reverted merge commit removed from main.

@samgutentag
Copy link
Copy Markdown
Member Author

Code verification (2026-06-01): 12 confirmed / 0 contradicted / 1 ambiguous / 0 unverifiable

Claim Verdict Source
upload, test, validate subcommands exist confirmed cli/src/main.rs:81-87
test runs the command; upload ingests existing results confirmed cli/src/main.rs:82-86
--bazel-bep-path flag confirmed cli/src/upload_command.rs:56
--junit-paths flag (comma-separated globs) confirmed cli/src/upload_command.rs:50
--org-url-slug flag confirmed cli/src/upload_command.rs:73
--token flag (defaults to TRUNK_API_TOKEN) confirmed cli/src/upload_command.rs:88
--repo-url flag overrides inferred remote confirmed cli/src/upload_command.rs:107
--test-process-exit-code flag confirmed cli/src/upload_command.rs:221
TRUNK_API_TOKEN env var confirmed constants/src/lib.rs:28
"No tests were found" message confirmed cli/src/upload_command.rs:903
validate "name too short" / "classname too short" warnings confirmed context/src/junit/validator.rs:692-718
SSH host-alias remote causes upload rejection confirmed context/src/repo/mod.rs:391
Binary BEP parses "more reliably" than JSON ambiguous cli/src/context.rs:565-589

Nothing contradicts source. Every flag, subcommand, env var, and error/warning string in the PR matches the analytics-cli code. One claim is ambiguous and worth a wording check before publishing: the "No tests found on upload" BEP guidance. The recommendation (produce a binary BEP via --build_event_binary_file and pass it to --bazel-bep-path) is sound, but the framing "parses more reliably from binary than JSON" is a simplification of the actual behavior. The CLI parses the file as JSON first and only falls back to the binary parser when the JSON parse errors. The same --bazel-bep-path flag accepts either format (the doc's example is correct), but note that the flag's own --help text literally says "JSON file," so a reader who trusts the help text might not realize binary is accepted. Consider softening "parses more reliably from binary" to "the binary BEP format avoids JSON parse errors that can drop test cases" so the doc matches the JSON-first / binary-fallback reality. Not a blocker.


Source #1 — upload / test / validate subcommands and their semantics (confirmed)

File: trunk-io/analytics-cli/cli/src/main.rs#L81-L87

enum Commands {
    /// Run a test command and upload results to Trunk Flaky Tests. Automatically detects and reports flaky tests.
    Test(TestArgs),
    /// Upload test results to Trunk Flaky Tests. Use this when you've already run tests and have result files.
    Upload(UploadArgs),
    /// Validate test report files for compatibility with Trunk Flaky Tests. Runs locally without uploading data.
    Validate(ValidateArgs),

Reasoning: All three subcommands the PR documents exist. The clap doc comments confirm the PR's Info callout: test runs the test command directly; upload is for when results already exist; validate runs locally. The PR's "upload will not re-run tests" framing matches the upload help text exactly.

Source #2 — upload flags (--bazel-bep-path, --junit-paths, --org-url-slug, --token, --repo-url, --test-process-exit-code) (confirmed)

File: trunk-io/analytics-cli/cli/src/upload_command.rs#L50-L107

    pub junit_paths: Vec<String>,          // --junit-paths (comma-separated globs)
    pub bazel_bep_path: Option<String>,    // --bazel-bep-path
    pub org_url_slug: String,              // --org-url-slug (env TRUNK_ORG_URL_SLUG)
    pub token: Option<String>,             // --token (env TRUNK_API_TOKEN)
    pub repo_url: Option<String>,          // --repo-url (env TRUNK_REPO_URL)
    pub test_process_exit_code: Option<i32>, // --test-process-exit-code (line 221)

Reasoning: Every flag used in the PR's code blocks is a real clap #[arg(long, ...)] field on UploadArgs. Casing (kebab-case on the CLI from the snake_case field) matches the doc. --token help text says "Defaults to TRUNK_API_TOKEN env var," confirming the env-var usage in the PR's BEP example.

Source #3 — TRUNK_API_TOKEN env var (confirmed)

File: trunk-io/analytics-cli/constants/src/lib.rs#L28

pub const TRUNK_API_TOKEN_ENV: &str = "TRUNK_API_TOKEN";

Reasoning: The PR uses $TRUNK_API_TOKEN in two code blocks. The constant resolves to exactly that string and is wired to --token via env = constants::TRUNK_API_TOKEN_ENV. (Note: this is also the var the prior PR renamed from TRUNK_TOKEN, so the PR is on the current name.)

Source #4 — "No tests were found" message (confirmed)

File: trunk-io/analytics-cli/cli/src/upload_command.rs#L903

                "⚠️  No tests were found in the provided test results",

Reasoning: The accordion title quotes "No tests were found" and the prose says "no tests found." The real message is ⚠️ No tests were found in the provided test results. The PR is using a partial quote of a real string, not a fabricated one, so the user-facing wording will match what they see in CI logs. No change needed.

Source #5 — validate "name too short" / "classname too short" warnings (confirmed)

File: trunk-io/analytics-cli/context/src/junit/validator.rs#L692-L718

    #[error("test case classname too short")]
    TestCaseClassnameTooShort(String),
    ...
    #[error("test case name too short")]
    TestCaseNameTooShort(String),

Reasoning: Both warnings the PR documents exist verbatim as validation issues. They live in the junit validator (the validate command), and the PR's claim that "the same checks run server-side on upload" is consistent with these being shared context crate validators. The doc's quoted phrases ("test case name too short", "classname too short") are exact substrings of the real messages.

Source #6 — SSH host-alias remote causes 403 (confirmed)

File: trunk-io/analytics-cli/context/src/repo/mod.rs#L391-L424

        let re2 = Regex::new(r"^([^/]*?@)([^/]*):(.+)/([^/]+)")?;
        ...
        let domain = caps.get(2).map(|m| m.as_str()).unwrap_or(""); // host = text between @ and :
        ...
        let host = parts.0.trim().to_string();

Reasoning: For an SCP-style remote (git@host:owner/repo.git), re2 captures the host as whatever sits between @ and :. A remote like git@github-most:org/repo.git yields host github-most, not github.com, so the upload carries an unrecognized host and is rejected. Passing --repo-url git@github.com:org/repo.git routes through this same from_url (called at mod.rs:195), fixing the host. The PR's explanation and fix are accurate.

Source #7 — binary BEP "more reliable than JSON" (ambiguous)

File: trunk-io/analytics-cli/cli/src/context.rs#L565-L589

pub fn fall_back_to_binary_parse(
    json_parse_result: anyhow::Result<BepParseResult>,
    bazel_bep_path: &String,
) -> anyhow::Result<BepParseResult> {
    let mut binary_parser = BazelBepBinParser::new(bazel_bep_path);
    match json_parse_result {
        anyhow::Result::Ok(result) if !result.errors.is_empty() => {
            let binary_result = binary_parser.parse();   // only on JSON errors
            ...
        }
        anyhow::Result::Err(json_error) => {
            let binary_result = binary_parser.parse();   // only on JSON failure
            ...
        }
        just_json => just_json,                          // JSON wins when clean
    }
}

Reasoning: The --bazel-bep-path flag is always parsed as JSON first (BazelBepParser, serde_json::Deserializer), and the binary parser (BazelBepBinParser, decode_length_delimited) runs only as a fallback when the JSON parse returns errors or fails. So binary does succeed in cases where JSON drops test cases, which supports the PR's recommendation, but the literal claim "parses more reliably from binary than JSON" oversimplifies a JSON-first / binary-fallback path. The same flag accepts both formats (the PR's --bazel-bep-path bep.bin example is correct), but the flag's --help text says "JSON file," which could confuse a reader. Suggested softening: "use the binary BEP format to avoid JSON parse errors that can silently drop test cases." Not a blocker.

@samgutentag samgutentag added the code-verified-partial verify-docs-against-code: confirmed claims, some unverifiable. label Jun 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-verified-partial verify-docs-against-code: confirmed claims, some unverifiable.

Development

Successfully merging this pull request may close these issues.

1 participant