Skip to content

Refactor onboarding and testdrive orchestration - #309

Open
juan-fernandez wants to merge 4 commits into
mainfrom
juan-fernandez/refactor-onboard-testdrive
Open

juan-fernandez wants to merge 4 commits into
mainfrom
juan-fernandez/refactor-onboard-testdrive

Conversation

@juan-fernandez

@juan-fernandez juan-fernandez commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

What

Consolidate the CI flags and workflow loading shared by ddtest onboard and ddtest 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.

  1. From the PR checkout, build the binary and create a disposable Vitest repository:

    qa_checkout="$PWD"
    qa_root="$(mktemp -d)"
    go build -o "$qa_root/ddtest" .
    export DDTEST_QA_BIN="$qa_root/ddtest"
    mkdir -p "$qa_root/repo/.github/workflows"
    cd "$qa_root/repo"
    printf '%s\n' '{"name":"ddtest-refactor-qa","private":true,"type":"module","scripts":{"test":"vitest run"}}' > package.json
    printf '%s\n' 'export const add = (a, b) => a + b' > sum.js
    printf '%s\n' "import { expect, test } from 'vitest'; import { add } from './sum.js'; test('adds', () => expect(add(1, 2)).toBe(3));" > sum.test.js
    printf '%s\n' 'node_modules/' > .gitignore
    cat > .github/workflows/test.yml <<'YAML'
    name: tests
    on: [push]
    jobs:
      tests:
        runs-on: ubuntu-latest
        steps:
          - uses: actions/checkout@v4
          - uses: actions/setup-node@v4
            with: {node-version: '24'}
          - run: npm ci
          - run: npm test
    YAML
    npm install --save-dev --no-audit --no-fund vitest@4.1.7 @vitest/coverage-v8@4.1.7
    git init -b juan-fernandez/onboard-refactor-qa
    git add .
    git commit -S -m 'Create onboarding QA fixture'
  2. 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 --porcelain must remain empty after the preview. Run "$DDTEST_QA_BIN" onboard --apply --yes --agent and 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 report COMPLETE LOCALLY. Inspect both .testoptimization/testdrive.json and report.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 of COMPLETE LOCALLY. Step 3 must replace this check-only evidence with fresh full-execution results.

  3. 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. Change sum.js without 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's sum.js with git restore -- sum.js before continuing.

    Check that a rejected resume ID preserves existing evidence: copy .testoptimization/testdrive.json to "$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 with resume_not_found. Run cmp .testoptimization/testdrive.json "$qa_root/report-before-invalid-resume.json"; it must report no difference.

  4. Add a second provider and check the shared decision/flag path:

    cat > .gitlab-ci.yml <<'YAML'
    tests:
      image: node:24
      script:
        - npm ci
        - npm test
    YAML
    "$DDTEST_QA_BIN" onboard --agent
    "$DDTEST_QA_BIN" testdrive --all --agent --yes --format json
    "$DDTEST_QA_BIN" onboard --agent --ci-provider gitlab --ci-runtime node=24
    "$DDTEST_QA_BIN" onboard --instructions --ci-provider gitlab

    The first two commands must stop with ci_provider_required and offer GitHub/GitLab, without editing CI or starting tests. Explicit GitLab selection must remove that ambiguity, preserve --ci-provider gitlab and --ci-runtime node=24 in the next command, and produce GitLab setup instructions. The GitHub workflow and original tests must stay unchanged by these read-only commands.

  5. For each of onboard and testdrive, 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 structured invalid_options result, with no edits or test execution. Replace the fixture's GitHub workflow with printf 'jobs: [\n' > .github/workflows/test.yml; both onboard --instructions --ci-provider github and testdrive --all --agent --yes --format json --ci-provider github must fail with a workflow parse/discovery error rather than silently treating CI as absent. Explicit GitLab instructions must still be available.

  6. Check manual GitHub instructions in a separate disposable Jest fixture; no dependency installation is needed:

    mkdir -p "$qa_root/manual-instructions/.github/workflows"
    cd "$qa_root/manual-instructions"
    printf '%s\n' '{"private":true,"scripts":{"test":"jest"},"devDependencies":{"jest":"29.7.0"}}' > package.json
    git init -b juan-fernandez/instructions-refactor-qa
    cat > .github/workflows/test.yml <<'YAML'
    name: tests
    on: [push]
    jobs:
      tests:
        runs-on: ubuntu-latest
        env: ${{ fromJSON(vars.TEST_ENV) }}
        steps:
          - run: npm test
    YAML
    "$DDTEST_QA_BIN" onboard --instructions --ci-provider github --framework jest
    "$DDTEST_QA_BIN" testdrive --all --check-only --yes --agent --format json --ci-provider github --framework jest

    Manual instructions must exit successfully, identify .github/workflows/test.yml, and show the Jest setup guidance. The full configuration check must exit nonzero with configuration_check_failed and an environment-map decoding error; unresolved environment expressions must still block validation.

    Move the env line first to the workflow top level, then onto the run: npm test step, one placement at a time, and repeat both commands. Expect the same results for each placement. For each placement, add datadog/test-visibility-github-action@v3 with with: {languages: js} before the test step and repeat the instructions command: it must recognize existing setup and print the next testdrive command.

  7. 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, put datadog/test-visibility-github-action@v3 before the test command with languages: js and api_key: ${{ secrets.DD_API_KEY }}. Set js-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. Expect multiple_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.

  8. Review or copy the reports before cleanup. Return to "$qa_checkout", then remove only the disposable root created in step 1 with rm -rf -- "$qa_root".

Copy link
Copy Markdown
Contributor Author

Validation of commit 880482e:

  • make test: passed with the direct Node executable. The pre-existing Mocha worker-parent assertion fails when Node is launched through Volta’s extra parent process.
  • make lint: passed with the repository-pinned golangci-lint v2.15.0 built using Go 1.27.3; zero issues.
  • Full onboarding catalog with Codex: all 18 cases ran. 15 matched expectations, 1 improved, 1 was an agent failure, and 1 had an unexpected blocker identifier. Catalog exit status remains 1. Expected INCOMPLETE/FAILED cases count as matches, not successful onboarding.
Case Outcome Investigation
E08 / Cypress Agent failure The uncoached agent explicitly selected GitHub before asking the catalog’s scripted GitLab question, then encountered Chrome/OS/setup blockers and omitted the canonical handoff. Untouched 705adfe and the refactored binary return identical provider-choice and invalid-option JSON on a trusted, network-disabled fixture. The intended GitLab path was not validated by this trial.
E13 / TestNG Better than expected Two supported controls pass; four unavailable controls are explicitly unsupported rather than counted as successful. This capability policy already exists on the tested base and is unchanged by this refactor.
E17 / ScalaTest Unexpected blocker identifier The same unresolved release command moves from expected step 9 to actual step 10 after the agent inserts the prescribed setup action. Its cause, earlier blockers, paired results and six controls are unchanged.

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 705adfe; the only post-build source edit extended the existing language-boundary test to cover the moved CI files, and the full test/lint checks were rerun afterward.

@datadog-official

datadog-official Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Pipelines  Tests

🎉 All green!

🧪 All tests passed
❄️ No new flaky tests detected

🎯 Code Coverage (details)
• Patch Coverage: 91.29%
• Overall Coverage: 88.69% (+0.01%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: c764d46 | Docs | View more details | Give us feedback!

@juan-fernandez

Copy link
Copy Markdown
Contributor Author

/datadog autotest review
@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-05T23:03:37.714321Z c764d46 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: 880482ecda

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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".

@datadog-official datadog-official Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bits Code Review: FAIL

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.

Open Bits AI session

🤖 Bits Code Review · Commit 880482e

Comment thread internal/onboard/ci_provider_github.go Resolved
@juan-fernandez

Copy link
Copy Markdown
Contributor Author

/datadog autotest review
@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: 9e805eb840

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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".

@juan-fernandez
juan-fernandez marked this pull request as ready for review November 5, 2026 15:44
@juan-fernandez
juan-fernandez requested a review from a team as a code owner November 5, 2026 15:44

@datadog-official datadog-official Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bits Code Review: PASS

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.

Was this helpful? React 👍 or 👎

Open Bits AI session

🤖 Bits Code Review · Commit 9e805eb

@datadog-official datadog-official Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bits Code Review: PASS

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.

Was this helpful? React 👍 or 👎

Open Bits AI session

🤖 Bits Code Review · Commit 9e805eb

@juan-fernandez juan-fernandez changed the title Refactor shared CI handling for onboard and testdrive Refactor onboarding and testdrive orchestration Nov 5, 2026
@juan-fernandez
juan-fernandez force-pushed the juan-fernandez/refactor-onboard-testdrive branch from 5537d7d to a2cbbcc Compare November 5, 2026 18:30
@juan-fernandez

Copy link
Copy Markdown
Contributor Author

/datadog autotest review
@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: a2cbbccfaf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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".

@datadog-official datadog-official Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bits Code Review: PASS

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.

Was this helpful? React 👍 or 👎

Open Bits AI session

🤖 Bits Code Review · Commit a2cbbcc

@juan-fernandez

Copy link
Copy Markdown
Contributor Author

/datadog autotest review
@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: c764d46b95

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

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".

@datadog-official datadog-official Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bits Code Review: PASS

More details

Runtime-metadata requests to nodejs.org returned HTTP 502, leaving broader testdrive execution partially unverified despite completed static coverage.

Was this helpful? React 👍 or 👎

Open Bits AI session

🤖 Bits Code Review · Commit c764d46

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant