Repository navigation
Fix report-pipe scope, Java release selection and the Cypress handoff found by the Claude eval - #306
Conversation
A CI step that only converts the tests' report, such as cat report.json | cucumber-junit > junit.xml after the Cucumber command, was unresolved scope: cucumber-junit was an unknown executable that might start tests, which made CI runtime and tracer agreement inconclusive (E04). Report converters, mergers and viewers (cucumber-junit, junit-merge, jrm, mochawesome-merge, marge, xunit-viewer, allure, junit2html...) read result files and write a report, so ddtest now reads them as non-test tools in every language's CI. A step after the job's last test that runs only such tools becomes a caveat through the existing post-test rule; unknown tools, and scoped packages of the same name, still may start tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
onboard --apply refused cucumber-js --format json | cucumber-junit > x.xml as a compound command and told the agent to run the tests as a command of their own, leaving it to improvise the conversion. NYC, which the scoped command adds, also prints its summary to standard output, so the pipe can't stay. For a pipe from one Cucumber formatter on standard output into commands that can't start tests, ddtest now returns ci_edit_required with the two exact commands: the scoped test command writing the same report to a file (--format json:cucumber-report.json), then cat of that file into the rest of the pipeline. Rerunning onboard --apply accepts both. Other compound commands keep the general advice, which now explains the same shape. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Java setups printed <exact dd-java-agent release> and "such as 1.67.1", so agents picked the release themselves (1.68.1 on GitLab and CircleCI, 1.67.1 on GitHub) and validation depended on the agent and the date. ddtest now reads the latest release from Maven Central's dd-java-agent metadata once per run, where a Java setup is first written, and names it in the GitLab, CircleCI, Buildkite and Jenkins snippets, the setup without Bash or curl, the GitHub action instructions, the hand-written download and the Jenkins plugin pin. An exact --tracer-version wins and a floating one is refused; the CI-pinned release stays the one validated, and the Jenkins container snippet still follows the host stages' plugin pin. javaRecommendedTracer remains only as the offline fallback. Tests inject the resolver and never reach the network. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
For a framework onboard --apply doesn't set up (Cypress), the result said "framework-specific CI loader/hook integration", and agents improvised a hook before finding the instructions (E08). The result now returns next_command onboard --framework <name> --instructions, as unresolved scope does, names the tracer setting with the selected release, forbids a hand-made loader or hook, and keeps resume_command for validation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: d3d6df7 | 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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f56c3d798
ℹ️ 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-prod-us1-5[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
The prescribed Cucumber split skips JUnit report conversion when tests fail. The Cypress handoff also loses an explicitly selected Datadog site, so following its instructions can configure CI for a different site than validation.
🤖 Bits Code Review · Commit 6f56c3d
With --site datadoghq.eu (any non-US1 site), the Cypress ci_edit_unsupported handoff sent the agent to onboard --instructions, which renders the setup for datadoghq.com, and told it to apply that setup exactly as shown, while resume_command kept the selected site. onboard --instructions takes no --site, like the tracer release it leaves as a placeholder, so the handoff now names the site the same way: "the Datadog site datadoghq.eu in place of the example site datadoghq.com, and dd-trace X in place of its tracer release placeholder", in reason, needs and required_input. The unresolved-scope handoff and the manual CI hook handoffs (ci_edit_unsupported, ci_edit_required), which also point at onboard --instructions, name a non-default site for setup written by hand too. The default site needs no substitution and adds nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The prescribed split of cucumber-js --format json | cucumber-junit > x.xml into two CI commands lost the report when scenarios failed: GitLab, CircleCI, Buildkite and Jenkins stop at the first failing script command, so the conversion never ran. The original pipe always converted; with pipefail (GitLab, CircleCI) it failed on failing tests, and without it (Buildkite, Jenkins sh) it took the converter's status and hid them. ddtest now prescribes one CI command that converts the report whatever the result and exits with the tests' status: (status=0; <scoped tests> || status=$?; cat cucumber-report.json | <converter>; exit "$status") The || keeps errexit from stopping at failing tests, and the subshell keeps the exit to this command, so a passing run continues with the script's later commands. Buildkite gets it with $$ escapes. Every provider's reader now takes that exact subshell as its tests (the configuration, with the scoped prefix and NYC read as before) plus the commands before the exit, which resolution checks can't start the tests and records as a step note outside validation. Any other shape, a conversion that may start tests, or an exit with another value stays unresolved. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/datadog autotest review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d891662e2
ℹ️ 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".
There was a problem hiding this comment.
Report-pipe handling can prescribe edits that remove conversion commands or corrupt a report by aliasing its input and output. It also hides preparation needed by later tests and rejects valid quoted formatter options.
🤖 Bits Code Review · Commit 5d89166
Review follow-ups on the status-preserving subshell that onboarding prescribes for a Cucumber report pipe, and on the pipe rewrite itself. - Each command between the tests and the saved-status exit must fall through to it: one foreground command, or a pipeline of them, whose literal program is no flow-changing builtin (exit, return, exec, break, continue, set, eval, source, trap, ...), with no && or || list and no read or assignment of the status variable. Otherwise the subshell is not read and the step stays unresolved, so (status=0; cucumber-js || status=$?; exit 0; exit "$status") no longer passes with its failures masked. - Those commands are now resolved as the same commands after the tests in a plain sequence: a file change such as cp ci.env .env stays setup for a later test step, and a command ddtest can't resolve is reopened when a later test step follows it. The check that rejects a tail that may start the tests still runs first. - When the tests inside an existing subshell still need the scoped prefix or NYC, the prescribed command is the whole subshell with only its tests edited, keeping the conversion and the exit with the tests' status. - The intermediate report takes a numeric suffix until the pipeline names it nowhere, so cucumber-js --format junit | tee cucumber-report.xml reads from cucumber-report-1.xml instead of aliasing its input and output. - A quoted formatter option such as '--format=json' is rewritten from its decoded text and quoted only when needed, so the prescription parses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/datadog autotest review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6987d5dfc0
ℹ️ 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".
There was a problem hiding this comment.
Cucumber report pipelines ending with a semicolon or inline comment produce syntactically invalid onboarding replacements. Bounding the converter to its parsed statement fixes the reproduced failures while preserving ordinary pipelines.
🤖 Bits Code Review · Commit 6987d5d
The prescribed status subshell now records the conversion's failure when
the tests pass, and keeps the tests' status when both fail:
(status=0; <tests> || status=$?; <conversion> || { converted=$?; [ "$status" -ne 0 ] || status=$converted; }; exit "$status")
so a missing or broken report fails the step with errexit off too, as the
original pipe did. The conversion pipe keeps the CI shell's own pipefail
setting, as the original pipe's status did. ddtest reads only that guarded
form back.
ddtest also prescribes only a command it reads back as the same tests and
conversion, falling back to the general compound-command advice otherwise
(a conversion through a { ...; } group, or one that reads the status
variable), and the conversion now ends at the pipeline's last stage, so a
trailing ; or comment no longer breaks the prescription.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/datadog autotest review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dec93ebe8f
ℹ️ 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".
There was a problem hiding this comment.
The new wrapper reader accepts variable-writing builtins that erase a saved test failure. Consequently, scope discovery can accept CI commands as preserving test failures even though they exit successfully.
🤖 Bits Code Review · Commit dec93eb
A status subshell's commands between its tests and its exit may now start any external program, but of the shell's builtins only those that print or test (:, [, echo, false, printf without options, pwd, test, true). read, printf -v, mapfile, getopts and the like write the saved status through a literal argument, which the assignment check didn't see, so a command that masks failing tests stays unresolved. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The report a split Cucumber pipe writes before its conversion now also skips names that are a file, or a path git tracks, in the directory the CI command runs in, so the tests never overwrite a fixture such as a tracked cucumber-report.json. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
For Java, --tracer-version latest now selects the exact latest dd-java-agent release, as no --tracer-version does, and the onboarding handoff and testdrive carry that release: the setup pins it, so the resumed validation compares the same release with the CI pin instead of reporting the literal latest as a mismatch. Other languages keep latest, which their registry resolves when the tracer installs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/datadog autotest review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83d3f9dedc
ℹ️ 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".
There was a problem hiding this comment.
Bash’s printf %n can overwrite the saved test status, and the new wrapper reader accepts that command as status-preserving. This allows a CI wrapper that hides failed tests to be accepted.
🤖 Bits Code Review · Commit 83d3f9d
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83d3f9dedc
ℹ️ 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".
There was a problem hiding this comment.
More details
Broader testdrive behavior remains incompletely verified because nodejs.org metadata requests returned HTTP 502; focused verification of the changed onboarding paths succeeded.
🤖 Bits Code Review · Commit 83d3f9d
The report-pipe prescription now removes the intermediate report inside the status subshell before the tests, so Cucumber failing before it writes the report leaves cat a missing file instead of converting an earlier run's report. The CI reader accepts only that rm -f of the file the tests write and the conversion reads. printf is no longer a builtin allowed between the tests and the exit: bash's printf writes a variable through -v or a %n in its format. External programs, /usr/bin/printf included, stay allowed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A report pipe whose tests or conversion read a shell variable named status or converted got a prescription that shadowed it with the subshell's own status. The prescription now records into status and converted only when the pipe references neither, and otherwise into datadog_status and datadog_converted, then those with a numeric suffix. The reader rejects a status subshell whose tests read its status variable or a saved conversion status, so the old shadowing form stays unresolved. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/datadog autotest review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c988f33c6e
ℹ️ 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".
There was a problem hiding this comment.
More details
The completed static review produced no reportable findings. Broader testdrive validation was limited by HTTP 502 responses from nodejs.org.
🤖 Bits Code Review · Commit c988f33
…holds them
A container stage's report-pipe split, status-subshell edit and direct
scoped command were handed off as raw shell, which Groovy interpolates
($status, ${NODE_OPTIONS}) or rejects ($?) when pasted into a
double-quoted sh string. They now come as sh '''…''' holds them, with
backslashes and runs of three quotes escaped, and say to replace a
double-quoted script with sh '''…''', as the setup snippet's
instructions already do.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The real-shell checks of the status subshell started one shell from the test per case, and one bash -n and dash -n per prescription. Each shell now runs all of a test's cases from one process, each case still in a fresh shell and directory of its own, and one bash -n and one dash -n parse every prescription, each the body of its own function. sh runs every combination of errexit, the tests' result and the conversion's; bash, with pipefail, and dash run a smoke subset where the tests, the conversion or both fail. Each provider reaches the reader's rejections of a removal and of an unsafe builtin through two representative forms, one that Buildkite escapes, since the reader tables check every form; the reader table gains rm -rf ., the one form only the provider loop had. The fresh report name tests run on one provider: the name doesn't depend on it, and every provider's escaping keeps its own end-to-end test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/datadog autotest review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4d9952489
ℹ️ 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".
There was a problem hiding this comment.
The new status-wrapper reader accepts a Bash test operand that overwrites the saved exit status, allowing it to approve a CI command that returns success after failing tests.
🤖 Bits Code Review · Commit e4d9952
…eral
A container stage's report-pipe split, status-subshell edit and direct
scoped command came escaped for sh '''…''', with an instruction to switch
a double-quoted script to it. In a GString that also interpolates other
expressions, such as echo ${env.BUILD_NUMBER}, the switch handed the
shell ${env.BUILD_NUMBER}, a bad substitution.
The prescription now follows the literal type Groovy's lexer read for the
step's script: in "…" or """…""" each backslash and $ is escaped (and in
"…" each double quote and line break), in '…' each backslash, single
quote and line break, and in '''…''' each backslash and run of three
quotes. The user replaces only the test command inside the existing
string, whose other expressions stay interpolated.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rands bash evaluates an array subscript in some test operands, so test -v 'BASH_VERSINFO[status=0]' after the tests reset the saved status and the subshell exited 0, while the reader read it as status-preserving. test and [ now qualify only as a file test, -n or -z on a literal, = or != between literals, or a comparison of literal integers, each operand a literal without $, a backslash, [ or a glob character. -v, -o/-a and ! combinations, and any other operand set, leave the subshell unresolved. The guard's own [ "$status" -ne 0 ] is a separate shape, still read. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What
cat report.json | cucumber-junit > junit.xml, junit-merge, jrm, mochawesome-merge, marge, xunit-viewer, allure, junit2html…) is now a caveat, not unresolved CI scope.cucumber-js --format json | cucumber-junit > x.xml),onboard --applyreturnsci_edit_requiredwith the one exact command to write:(status=0; rm -f cucumber-report.json; <scoped tests writing --format json:cucumber-report.json> || status=$?; cat cucumber-report.json | <rest of the pipe> || { converted=$?; [ "$status" -ne 0 ] || status=$converted; }; exit "$status"). It converts the report whether the tests pass or fail, first removing any earlier intermediate report so a run that fails before writing one never converts a stale report, and fails the step with the tests' status, or the conversion's when only the conversion fails, with errexit on or off. ddtest prescribes it only after reading it back with its own CI reader. Rerunningonboard --applyaccepts it. The intermediate report gets a name used neither in the pipe nor by a file in the step's directory (cucumber-report-1.jsonand so on), and only external programs or the builtins:,[,test,echo,true,false,pwdmay run after the tests in that command (noprintfbuiltin). On Jenkins, the command comes escaped for the string literal theshstep already uses (\$and\\in a double-quoted GString, plus\"in"…";\\in a single-quoted one, plus\'in'…'), so you replace only the test command inside that string and its other${…}interpolations keep working. After the tests,test/[may only run as a file test,-n/-z,=/!=or a literal-integer comparison.--tracer-version) instead of<exact dd-java-agent release>. This covers GitLab, CircleCI (orb and no-Bash setup), Buildkite, Jenkins, the GitHub action instructions, hand-written downloads and the Jenkins plugin pin. A floating--tracer-versionis refused. ddtest falls back to 1.67.1 only when Maven Central can't be read. An explicit--tracer-version latestresolves to that exact release, which the handoff and testdrive carry.ci_edit_unsupportednow returnsnext_command: onboard --framework cypress --instructionsand names the release to write, in place of "framework-specific CI loader/hook integration". With a non-default--site, the handoff also names that site in place of the exampledatadoghq.com, as do the other handoffs that point atonboard --instructions.Why
The first Claude run of the onboarding eval catalog found three issues:
onboard --instructions.E2E testing
Prerequisites: Go from
go.mod, Node 24 with npm, git, network access to GitHub, npm and Maven Central. Build this branch withmake buildand putddtestonPATH. Work in an empty scratch folder."code":"ci_edit_required", andreasonsaysReplace this CI command with exactly this one, which removes any earlier intermediate report, writes the report to a file, converts it whether the tests pass or fail, and exits with the tests' status, or the conversion's when only the conversion fails: (status=0; rm -f cucumber-report.json; NODE_OPTIONS="${NODE_OPTIONS:-} ${DD_CUCUMBER_NODE_OPTIONS:-}" DD_CIVISIBILITY_IMPACTED_TESTS_DETECTION_ENABLED=false npx --no-install nyc -- ./node_modules/.bin/cucumber-js ./GameController_ATDD --format json:cucumber-report.json || status=$?; cat cucumber-report.json | ./node_modules/.bin/cucumber-junit > testresults_cucumber.xml || { converted=$?; [ "$status" -ne 0 ] || status=$converted; }; exit "$status")..gitlab-ci.yml, replace theacceptance testscript line with that command as a single-entry. Then runddtest onboard --framework cucumber --apply --yes --ci-provider gitlab --agent --allow-dirty.Expected:
"status":"complete_locally". In.testoptimization/testdrive.json,summary.statusisCOMPLETE LOCALLY,ci_configurationiscompatible, andscope.commands[0].step_scope_notesincludes "This CI step runs cat cucumber-report.json | ./node_modules/.bin/cucumber-junit > testresults_cucumber.xml after its cucumber tests, whether they pass or fail, then exits with the tests' status, or with the status of a command after them that fails when they pass." (plus a note that thecatpart can't start cucumber).GameController_ATDD/zz_fail.featurewith a scenario whose step is undefined, then runbash -eo pipefail -c '<the command from step 1>; echo next'. Expected: exit status 1,testresults_cucumber.xmlwritten, and nonextprinted. Remove the feature and rerun: exit 0 andnextprinted. With a passing suite and the converter replaced bysh -c 'cat > testresults_cucumber.xml; exit 5',bash -c(without-e) exits 5; with the failing feature as well, it exits 1../convert.sh(an unknown script), then runddtest testdrive --framework cucumber --ci-provider gitlab --check-only --agent --yes. Expected:scope.unresolvedin.testoptimization/testdrive.jsonnames the step: "CI runs cat cucumber-report.json | ./convert.sh > testresults_cucumber.xml after the tests in the same command, and it may start cucumber itself…". Also putexit 0;beforeexit "$status"and rerun: the acceptance-test step is unresolved and no cucumber command is accepted. Withread status <<<0(orprintf -v status 0) as the converter, the step is also unresolved. With an existingcucumber-report.jsonin the repository root, the prescription usescucumber-report-1.jsonand leaves the file untouched.JAVA_TRACER_VERSION="<release>", with the release thatcurlprinted (1.68.1 today), and the note saysThe setup installs dd-java-agent <release>, the exact release ddtest selected. No<exact dd-java-agent release>placeholder is left.ddtest onboard --framework junit5 --ci-provider gitlab --agent --tracer-version 1.67.1. Expected: the instruction hasJAVA_TRACER_VERSION=\"1.67.1\", andresume_commandcarries--tracer-version 1.67.1. Then run it with--tracer-version 1.67. Expected:"code":"invalid_options"withthe Java tracer version must be an exact dd-java-agent release, such as 1.67.1, or latest: 1.67. Then run it with--tracer-version latest: the instruction pinsJAVA_TRACER_VERSIONto the current Maven Central release, andresume_commandcarries--tracer-version <that release>, notlatest.DATADOG_JENKINS_PLUGIN_TRACER_DISTRIBUTION_URLto 1.66.0 and that also has a container stage,ddtest onboard --apply --yeswritesDD_SET_TRACER_VERSION_JAVA="1.66.0"in the container snippet."code":"ci_edit_unsupported"and"next_action":"run_command", withnext_commandending inonboard --framework cypress --instructions --ci-provider gitlab.reasonsaysonboard --apply does not write the Cypress CI setup on GitLab CI. next_command prints it: apply that setup exactly as shown, with dd-trace 6.23.0 in place of its tracer release placeholder (JS_TRACER_VERSION: 6.23.0)(the release is the one ddtest selects that day).resume_commandistestdrive ... --tracer-version <same release> ... --resume <id>. Runningnext_commandprints the GitLab setup entry.Rerun with
--site datadoghq.eu. Expected:reason,needsandrequired_inputcontain "the Datadog site datadoghq.eu in place of the example site datadoghq.com, and dd-trace in place of its tracer release placeholder";next_commandhas no--site, andresume_commandhas--site datadoghq.eu.🤖 Generated with Claude Code