Repository navigation
Refactor onboarding and testdrive orchestration - #309
juan-fernandez wants to merge 4 commits into
Conversation
|
Validation of commit
No catalog expectations were changed to make the run green. No code regression was identified in the completed comparisons, but the catalog sweep is not entirely green for the reasons above. All 18 trial containers/process trees and owned trial folders were removed. Per-case assessments, diffs, public activity, final answers and JSON reports were retained locally. Spock’s HTML report exceeded the evaluator’s retention limit, so that HTML could not be independently reviewed; its JSON was retained. Actual CI execution and Datadog backend processing were not exercised. The evaluated source snapshot is the code in this commit, based on |
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: c764d46 | Docs | View more details | Give us feedback! |
|
/datadog autotest review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
The shared loader introduced in this PR strictly decodes job-level env fields as map[string]string, breaking onboard --instructions for any GitHub workflow that uses a YAML expression (e.g., ${{ matrix.env }}) as the environment value — a case the previous manual parser silently ignored. The loader/model boundary must be adjusted to restore tolerant decoding for the manual-instructions path without weakening full-validation checks.
🤖 Bits Code Review · Commit 880482e
|
/datadog autotest review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
More details
Shared CI option resolution preserves provider-before-statement validation and structured error rendering. Full testdrive validation remains limited by HTTP 502 responses from nodejs.org.
🤖 Bits Code Review · Commit 9e805eb
There was a problem hiding this comment.
More details
The extracted CI and testdrive helpers preserve existing behavior according to the completed static review and targeted package validation. Full-suite and actual CI execution remain unverified.
🤖 Bits Code Review · Commit 9e805eb
5537d7d to
a2cbbcc
Compare
|
/datadog autotest review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
More details
Full testdrive validation was limited by HTTP 502 responses from nodejs.org during Node metadata fetches; focused onboarding, command-matching, CLI, and workflow checks succeeded.
🤖 Bits Code Review · Commit a2cbbcc
|
/datadog autotest review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
More details
Runtime-metadata requests to nodejs.org returned HTTP 502, leaving broader testdrive execution partially unverified despite completed static coverage.
🤖 Bits Code Review · Commit c764d46
What
Consolidate the CI flags and workflow loading shared by
ddtest onboardandddtest testdrive, with focused owners for provider selection, setup and discovery. Manual GitHub instructions decode only the fields they inspect, so expression-valued environments still allow guidance while full validation keeps its strict workflow model.Give each testdrive invocation an explicit owner for its result, checkpoint and recovery state. The coordinator sequences the phases; focused helpers handle facts, setup failures and final handoffs. Coordinated and standalone validation share CI questions and pinned-tracer selection.
Separate onboarding handoffs from the shared protocol renderer, executable lookup and setup-fact schemas. Document the boundaries in
internal/onboard/README.md, and cover shared loading, command matching, environment expressions and conflicting tracer pins with regression tests. Commands, JSON schemas and validation rules retain their existing behavior.Shared paired execution, preflight evidence, CI configuration checks and native-result comparison now live in framework-neutral files instead of Jest adapters. Frameworks register preflight methods directly; one binding point preserves the selected drive and execution context.
Why
The two commands had accumulated duplicate CI handling and large files that mixed discovery, provider selection, setup and validation. The workflow coordinator also managed retry state through a large closure, while coordinated and standalone validation repeated the same CI decisions. That made behavior difficult to locate and recovery changes difficult to review.
Shared decision helpers and explicit ownership make the code easier to navigate and reduce the chance that validation paths drift apart. The shared validation engine also had grown inside Jest files even though every supported framework uses it, obscuring ownership and repeating registration wrappers.
E2E testing
Prerequisites: this PR checked out, the Go version in
go.mod, Node 24, npm, Git with commit signing configured, package-registry access, writable temporary storage and loopback listeners. No Datadog API key is needed. Keep the fixture local; do not push it or run real CI.From the PR checkout, build the binary and create a disposable Vitest repository:
Run
"$DDTEST_QA_BIN" onboard --agent, then"$DDTEST_QA_BIN" onboard --apply. Expect GitHub/Vitest discovery and a preview naming the setup edits;git status --porcelainmust remain empty after the preview. Run"$DDTEST_QA_BIN" onboard --apply --yes --agentand follow any exact prerequisite command it returns. Expect the original test to remain unchanged, recognized CI setup and report exclusions to be applied, and full local validation to reportCOMPLETE LOCALLY. Inspect both.testoptimization/testdrive.jsonandreport.html; baseline/reporting outcomes must agree, supported feature controls must pass, and actual CI/backend execution must remain explicitly untested.Also run
"$DDTEST_QA_BIN" testdrive --check-only --yes --format json --framework vitest. Expect a ready configuration-check result, with no paired execution or feature controls and no claim ofCOMPLETE LOCALLY. Step 3 must replace this check-only evidence with fresh full-execution results.Run
"$DDTEST_QA_BIN" testdrive --all --agent --yes --format json --timeout 2m, then"$DDTEST_QA_BIN" testdrive --report --format json. Expect the same selected CI configuration and current full-execution evidence. Changesum.jswithout rerunning tests and invoke the report command again: it must report stale evidence with a full-validation next command, rather than certify the edit. Restore only the fixture'ssum.jswithgit restore -- sum.jsbefore continuing.Check that a rejected resume ID preserves existing evidence: copy
.testoptimization/testdrive.jsonto"$qa_root/report-before-invalid-resume.json", then run"$DDTEST_QA_BIN" testdrive --all --agent --yes --format json --ci-provider github --resume 00000000000000000000000000000000. Expect a nonzero exit withresume_not_found. Runcmp .testoptimization/testdrive.json "$qa_root/report-before-invalid-resume.json"; it must report no difference.Add a second provider and check the shared decision/flag path:
The first two commands must stop with
ci_provider_requiredand offer GitHub/GitLab, without editing CI or starting tests. Explicit GitLab selection must remove that ambiguity, preserve--ci-provider gitlaband--ci-runtime node=24in the next command, and produce GitLab setup instructions. The GitHub workflow and original tests must stay unchanged by these read-only commands.For each of
onboardandtestdrive, run"$DDTEST_QA_BIN" <command> --agent --format json --ci-provider unknown, then"$DDTEST_QA_BIN" <command> --agent --format json --ci-provider github --ci-runtime node=lts. Expect a nonzero exit and one structuredinvalid_optionsresult, with no edits or test execution. Replace the fixture's GitHub workflow withprintf 'jobs: [\n' > .github/workflows/test.yml; bothonboard --instructions --ci-provider githubandtestdrive --all --agent --yes --format json --ci-provider githubmust fail with a workflow parse/discovery error rather than silently treating CI as absent. Explicit GitLab instructions must still be available.Check manual GitHub instructions in a separate disposable Jest fixture; no dependency installation is needed:
Manual instructions must exit successfully, identify
.github/workflows/test.yml, and show the Jest setup guidance. The full configuration check must exit nonzero withconfiguration_check_failedand an environment-map decoding error; unresolved environment expressions must still block validation.Move the
envline first to the workflow top level, then onto therun: npm teststep, one placement at a time, and repeat both commands. Expect the same results for each placement. For each placement, adddatadog/test-visibility-github-action@v3withwith: {languages: js}before the test step and repeat the instructions command: it must recognize existing setup and print the next testdrive command.Return to
"$qa_root/repo"and replace its malformed GitHub workflow with the valid test job from step 1, duplicated as two jobs. In each job, putdatadog/test-visibility-github-action@v3before the test command withlanguages: jsandapi_key: ${{ secrets.DD_API_KEY }}. Setjs-tracer-version: '6.20.1'in the first job and'6.24.0'in the second. Run"$DDTEST_QA_BIN" testdrive --framework vitest --all --agent --yes --format json --ci-provider github, without an explicit--tracer-version. Expectmultiple_ci_tracers, a location naming the conflicting job, and a request to align the intended releases or use separately scoped runs. It must stop before executing tests or choosing either release silently.Review or copy the reports before cleanup. Return to
"$qa_checkout", then remove only the disposable root created in step 1 withrm -rf -- "$qa_root".