diff --git a/internal/cmd/temp_main_test.go b/internal/cmd/temp_main_test.go index dded6438..7cb1e60d 100644 --- a/internal/cmd/temp_main_test.go +++ b/internal/cmd/temp_main_test.go @@ -1,14 +1,19 @@ package cmd import ( + "context" "fmt" "os" "testing" + + "github.com/DataDog/ddtest/internal/onboard" ) // Commands under test create workflow locks and checkpoints under the // temporary folder; keep them out of the user's real one. func TestMain(m *testing.M) { + // Java setups name a fixed latest dd-java-agent release, read offline. + onboard.SetJavaReleaseResolver(func(context.Context) (string, error) { return "1.68.1", nil }) if os.Getenv("DDTEST_ISOLATED_TEST_TMP") == "1" { os.Exit(m.Run()) } diff --git a/internal/onboard/agent.go b/internal/onboard/agent.go index cb9f828a..9442dc5f 100644 --- a/internal/onboard/agent.go +++ b/internal/onboard/agent.go @@ -224,6 +224,11 @@ func StartWithOptions(output io.Writer, options StartOptions) error { if err != nil { return fail("framework_unresolved", err) } + selected, err := SelectTracerRelease(detected.Name(), options.Tracer) + if err != nil { + return errors.Join(err, WriteAgentResult(output, AgentResult{Status: "needs_input", Phase: "discovery", Code: "invalid_options", Reason: err.Error(), NextAction: "supply_input"})) + } + options.Tracer = selected if root, err := os.Getwd(); err == nil { if err := CheckCIProviderSupport(root, detected.Name(), frameworkName); err != nil { var choice *CIProviderChoice diff --git a/internal/onboard/apply_plugin.go b/internal/onboard/apply_plugin.go index 604ac3e9..03ee2916 100644 --- a/internal/onboard/apply_plugin.go +++ b/internal/onboard/apply_plugin.go @@ -57,6 +57,11 @@ func ApplyPluginSetup(output io.Writer, options StartOptions, apply PluginApplyO if err != nil { return start() } + selected, err := SelectTracerRelease(detected.Name(), options.Tracer) + if err != nil { + return start() // The handoff names the invalid selector. + } + options.Tracer = selected // The exact release the setup pins. framework, err := onboardingFramework(detected) if err != nil || CheckCIProviderSupport(root, detected.Name(), framework) != nil || RequiredCIRuntimes(context.Background(), root, detected.Name(), framework) != nil { return start() diff --git a/internal/onboard/apply_provider.go b/internal/onboard/apply_provider.go index cea12701..433cf596 100644 --- a/internal/onboard/apply_provider.go +++ b/internal/onboard/apply_provider.go @@ -213,12 +213,12 @@ func planProviderJavaScriptSetup(root string, provider ciProvider, workflows []c continue // Explicit browser runner remains outside instrumentation. } } - command, err := directScopedCommand(leaf.step, leaf.test, framework) + command, err := directScopedCommand(root, leaf.step, leaf.test, framework) if err != nil { - return nil, &SetupGap{location, err.Error()} + return nil, directScopedGap(provider, location, leaf.step, err) } if command != "" { - return nil, &EditThenApplyGap{SetupGap{directLeafLocation(location, leaf.step), directLeafReason(provider, framework, leaf.test.Command, command, setupIndex >= 0)}} + return nil, &EditThenApplyGap{SetupGap{directLeafLocation(location, leaf.step), directLeafReason(provider, leaf.step, framework, leaf.test.Command, command, setupIndex >= 0)}} } } if handWritten != nil { @@ -227,7 +227,7 @@ func planProviderJavaScriptSetup(root string, provider ciProvider, workflows []c } if framework == "cucumber" { // A direct invocation needs NYC in the CI command itself. - if gap := directCucumberCoverageGap(provider, location, workflow, job, direct, "the job's hand-written setup stays."); gap != nil { + if gap := directCucumberCoverageGap(root, provider, location, workflow, job, direct, "the job's hand-written setup stays."); gap != nil { return nil, gap } } @@ -284,7 +284,7 @@ func planProviderJavaScriptSetup(root string, provider ciProvider, workflows []c bare = append(bare, leaf) } } - if gap := directCucumberCoverageGap(provider, location, workflow, job, bare, "the job's "+provider.setupName()+" stays."); gap != nil { + if gap := directCucumberCoverageGap(root, provider, location, workflow, job, bare, "the job's "+provider.setupName()+" stays."); gap != nil { return nil, gap } } @@ -689,28 +689,110 @@ func cucumberLoaderReached(workflow ciWorkflow, job runtimeJob, steps []matchedT // without the scoped variable): no package script carries the wrapper // onboard --apply adds, so the CI command itself must. stays says what // happens to the job's setup. nil when every one has NYC. -func directCucumberCoverageGap(provider ciProvider, location string, workflow ciWorkflow, job runtimeJob, direct []directLeaf, stays string) error { +func directCucumberCoverageGap(root string, provider ciProvider, location string, workflow ciWorkflow, job runtimeJob, direct []directLeaf, stays string) error { for _, leaf := range direct { workaround := cucumberImpactedWorkaroundSet(workflow, job, leaf.step) if leaf.test.usesNYC && workaround { continue } - command, err := directScopedCommand(leaf.step, leaf.test, "cucumber") + command, err := directScopedCommand(root, leaf.step, leaf.test, "cucumber") if err != nil { - return &SetupGap{location, err.Error()} - } - if _, ok := provider.(buildkite); ok { - command = buildkiteEscape(command) // pipeline upload would replace each $. + return directScopedGap(provider, location, leaf.step, err) } + command, written := providerCommand(provider, leaf.step, command) reason := CucumberCoverageGap + " CI runs " + leaf.test.Command + " directly, not through a package script, so onboard --apply cannot wrap it with NYC." if leaf.test.usesNYC { reason = "CI runs " + leaf.test.Command + " directly, not through a package script, without " + cucumberImpactedWorkaround + ", which onboard --apply adds to the package scripts it scopes and dd-trace's Cucumber instrumentation needs, so onboard --apply cannot add it to this command." } - return &EditThenApplyGap{SetupGap{directLeafLocation(location, leaf.step), reason + " Change this CI command to exactly: " + command + ". Then rerun onboard --apply; " + stays}} + return &EditThenApplyGap{SetupGap{directLeafLocation(location, leaf.step), reason + " Change this CI command to exactly: " + command + "." + written + " Then rerun onboard --apply; " + stays}} } return nil } +// directScopedGap is the gap of a direct invocation whose scoped command +// directScopedCommand could not write: the exact command that replaces a +// report pipe it splits (see ReportPipeSplit), or the reason it writes none. +func directScopedGap(provider ciProvider, location string, step runtimeStep, err error) error { + var split *ReportPipeSplit + if !errors.As(err, &split) { + return &SetupGap{location, err.Error()} + } + escaped := *split + var written string + escaped.Command, written = providerCommand(provider, step, split.Command) + return &EditThenApplyGap{SetupGap{directLeafLocation(location, step), escaped.Error() + ": it adds the " + provider.setupName() + " before the tests when the job has none, and reads the conversion as outside the tests." + written}} +} + +// providerCommand is a shell command ddtest prescribes for step, written +// as the provider's configuration holds it, and how to place it there when +// that takes more than replacing the old command: Buildkite's pipeline +// upload would replace each $, so each is doubled; a Jenkinsfile holds it +// in the Groovy string literal of the step's sh script (see +// jenkinsCommand). +func providerCommand(provider ciProvider, step runtimeStep, command string) (string, string) { + switch provider.name() { + case buildkiteProvider: + return buildkiteEscape(command), "" + case jenkinsProvider: + origin, _ := step.origin.(jenkinsShellStep) + return jenkinsCommand(origin.quote, command) + } + return command, "" +} + +// jenkinsCommand is command escaped for the Groovy string literal, opened +// by quote, that holds an sh step's script, so that Groovy hands the shell +// exactly command when it replaces the old one inside that literal, which +// keeps its type: the script's other commands, and any ${…} a GString +// interpolates in them, stay as they are. In a GString, in double quotes +// or triple double quotes, each backslash and $ is escaped; in a string in +// single quotes or triple single quotes, each backslash. A one-line +// literal escapes each of its own quotes and each line break; a +// triple-quoted one, a run of three of its quotes and one that ends the +// command, which would close it. A script that is not a literal gets the +// command as the shell runs it. +func jenkinsCommand(quote, command string) (string, string) { + escapeQuotes := func(text string, quote byte) string { + var escaped strings.Builder + for i := 0; i < len(text); i++ { + run := i + for run < len(text) && text[run] == quote { + run++ + } + if run == i { + escaped.WriteByte(text[i]) + continue + } + for j := i; j < run; j++ { + if run-i >= 3 || run == len(text) { + escaped.WriteByte('\\') + } + escaped.WriteByte(quote) + } + i = run - 1 + } + return escaped.String() + } + var escaped, kind string + switch quote { + case `"`: + escaped = strings.NewReplacer(`\`, `\\`, "$", `\$`, `"`, `\"`, "\n", `\n`).Replace(command) + kind = "the sh step's double-quoted Groovy string (\"…\") holds it, with each backslash, $, double quote and line break escaped" + case `"""`: + escaped = escapeQuotes(strings.NewReplacer(`\`, `\\`, "$", `\$`).Replace(command), '"') + kind = "the sh step's triple-double-quoted Groovy string (\"\"\"…\"\"\") holds it, with each backslash and $ escaped, and any run of three double quotes" + case "'": + escaped = strings.NewReplacer(`\`, `\\`, "'", `\'`, "\n", `\n`).Replace(command) + kind = "the sh step's single-quoted Groovy string ('…') holds it, with each backslash, single quote and line break escaped" + case "'''": + escaped = escapeQuotes(strings.ReplaceAll(command, `\`, `\\`), '\'') + kind = "the sh step's triple-single-quoted Groovy string ('''…''') holds it, with each backslash escaped, and any run of three single quotes" + default: + return command, " This is the command as the shell runs it: the sh step's script is not a Groovy string literal ddtest reads, so write it there escaped as that string needs, so that Groovy hands the shell exactly this command." + } + return escaped, " This is the command as " + kind + ", so Groovy hands the shell exactly the command while the script's other commands, and anything that string interpolates in them, stay as they are: replace only the old command inside that string." +} + // cucumberImpactedWorkaroundSet reports whether a direct Cucumber // invocation runs with Cucumber's impacted-tests workaround // (cucumberImpactedWorkaround): right after its scoped prefix, or in the @@ -732,15 +814,13 @@ func directLeafLocation(location string, step runtimeStep) string { // directLeafReason prescribes the CI command a direct Cucumber or // WebdriverIO invocation needs (see directScopedCommand), written as the // provider's configuration holds it, and what onboard --apply does next. -func directLeafReason(provider ciProvider, framework, leaf, command string, configured bool) string { - if _, ok := provider.(buildkite); ok { - command = buildkiteEscape(command) // pipeline upload would replace each $. - } +func directLeafReason(provider ciProvider, step runtimeStep, framework, leaf, command string, configured bool) string { + command, written := providerCommand(provider, step, command) edit := "it adds the " + provider.setupName() + " before this command, with the command that exports " + scopedEnvironment(framework) + "." if configured { edit = "the job's " + provider.setupName() + " already exports " + scopedEnvironment(framework) + "." } - return "CI runs " + leaf + " directly, not through a package script, so the command itself must read the tracer's preload from " + scopedEnvironment(framework) + ", as a scoped package script does. Change this CI command to exactly: " + command + ". Then rerun onboard --apply: " + edit + return "CI runs " + leaf + " directly, not through a package script, so the command itself must read the tracer's preload from " + scopedEnvironment(framework) + ", as a scoped package script does. Change this CI command to exactly: " + command + "." + written + " Then rerun onboard --apply: " + edit } // outsideTestJobs reports whether the language tells that job runs none of diff --git a/internal/onboard/apply_scoped.go b/internal/onboard/apply_scoped.go index 246c516e..e95d398e 100644 --- a/internal/onboard/apply_scoped.go +++ b/internal/onboard/apply_scoped.go @@ -1,15 +1,21 @@ package onboard import ( + "bytes" "encoding/json" + "errors" "fmt" + "io/fs" "maps" "os" + "os/exec" "path/filepath" "regexp" "slices" + "strconv" "strings" + "github.com/kballard/go-shellquote" "mvdan.cc/sh/v3/syntax" ) @@ -43,8 +49,10 @@ func scopedEnvironment(framework string) string { // sets them for that command alone, so the rest is the step's command, and // its leaf consumes the variable as a scoped package script's does (see // markScopedLeaf). Only a command whose first -// call carries the prefix, alone or piped into others, qualifies. +// call carries the prefix, alone or piped into others, qualifies. The tests +// of a status-preserving subshell qualify too (see readStatusSubshell). func (s *runtimeStep) readScopedPrefix() { + s.readStatusSubshell() command := strings.TrimSpace(s.Run) variable := "" for _, candidate := range []string{"DD_CUCUMBER_NODE_OPTIONS", "DD_WDIO_NODE_OPTIONS"} { @@ -80,6 +88,395 @@ func (s *runtimeStep) readScopedPrefix() { s.Run, s.scopedVariable = rest, variable } +// readStatusSubshell reads a CI command that runs the tests, then commands +// that must run whatever the tests' result, and exits with the tests' +// status, or with a later command's when only that command fails, as +// onboarding prescribes for a report conversion (see ReportPipeSplit and +// statusSubshell): +// +// (status=0; rm -f report.json; cucumber-js --format json:report.json || status=$?; cat report.json | cucumber-junit > junit.xml || { converted=$?; [ "$status" -ne 0 ] || status=$converted; }; exit "$status") +// +// The rm -f before the tests is optional, and only that command qualifies +// there: it removes the report the tests write and the first command after +// them reads (see removedReport). +// The || keeps errexit from stopping at failing tests or a failing +// conversion, and the subshell keeps the exit to this command, so a passing +// run continues with the script's later commands, as a script stopping at +// errors does, and a failing one fails the step after the conversion. Run +// becomes the tests, afterTests the commands between them and the exit, +// which resolution checks can't start tests and reads as a plain sequence +// after them, and statusWrapper the text around the tests. Each of those +// commands must fall through to the exit (see fallsThrough) and record its +// failure as the prescription does (see guardedCommand), and the tests must +// reference neither the status variable nor a saved one, which the subshell +// shadows (see statusVariables); any other command stays as written. +func (s *runtimeStep) readStatusSubshell() { + run := strings.TrimSpace(s.Run) + file, ok := parseCIShell(run) + if !ok || len(file.Stmts) != 1 { + return + } + outer := file.Stmts[0] + subshell, isSubshell := outer.Cmd.(*syntax.Subshell) + if !isSubshell || !plainStmt(outer) || len(subshell.Stmts) < 4 { + return + } + stmts := subshell.Stmts + variable, initial := onlyAssignment(stmts[0]) + if variable == "" || initial.Lit() != "0" { + return + } + report := removedReport(stmts[1]) + if report != "" { + if len(stmts) < 5 { + return + } + stmts = append(stmts[:1:1], stmts[2:]...) + } + record, isBinary := stmts[1].Cmd.(*syntax.BinaryCmd) + if !plainStmt(stmts[1]) || !isBinary || record.Op != syntax.OrStmt { + return + } + tests, isCall := record.X.Cmd.(*syntax.CallExpr) + if !plainStmt(record.X) || !isCall || len(tests.Args) == 0 { + return + } + if name, value := onlyAssignment(record.Y); name != variable || !lastStatus(value) { + return + } + exit, isExit := stmts[len(stmts)-1].Cmd.(*syntax.CallExpr) + if !plainStmt(stmts[len(stmts)-1]) || !isExit || len(exit.Assigns) > 0 || len(exit.Args) != 2 || exit.Args[0].Lit() != "exit" || !readsOnly(exit.Args[1], variable) { + return + } + if report != "" && (!writesReport(tests, report) || !readsReport(stmts[2], report)) { + return + } + var after []string + for _, stmt := range stmts[2 : len(stmts)-1] { + command, saved := guardedCommand(stmt, variable) + if command == nil || referencesVariable(record.X, saved) { + return + } + after = append(after, stmtText(run, command)) + } + if referencesVariable(record.X, variable) { + // The tests would read the subshell's status, not the one CI set. + return + } + start, end := record.X.Pos().Offset(), record.X.End().Offset() + s.Run, s.afterTests, s.statusWrapper = stmtText(run, record.X), after, [2]string{run[:start], run[end:]} +} + +// removedReport is the file a plain rm -f statement removes, as +// statusSubshell writes it before the tests: one literal relative name with +// no glob or option, so the command removes only that file. It is "" for any +// other statement. +func removedReport(stmt *syntax.Stmt) string { + call, isCall := stmt.Cmd.(*syntax.CallExpr) + if !plainStmt(stmt) || !isCall || len(call.Assigns) > 0 || len(call.Args) != 3 || call.Args[0].Lit() != "rm" || call.Args[1].Lit() != "-f" { + return "" + } + report := call.Args[2].Lit() + if report == "" || strings.HasPrefix(report, "-") || strings.HasPrefix(report, "/") || strings.ContainsAny(report, "*?[]{}~\\") || slices.Contains(strings.Split(report, "/"), "..") { + return "" + } + return report +} + +// writesReport reports tests that name report as a formatter's output, as +// --format json:report does. +func writesReport(tests *syntax.CallExpr, report string) bool { + return slices.ContainsFunc(tests.Args[1:], func(arg *syntax.Word) bool { + word, literal := shellLiteralWord(arg) + return literal && strings.HasSuffix(word, ":"+report) + }) +} + +// readsReport reports a guarded conversion (see guardedCommand) whose pipe +// starts with cat report, as statusSubshell writes it. +func readsReport(stmt *syntax.Stmt, report string) bool { + guard, isBinary := stmt.Cmd.(*syntax.BinaryCmd) + if !isBinary || guard.Op != syntax.OrStmt { + return false + } + first := guard.X + for { + pipe, isPipe := first.Cmd.(*syntax.BinaryCmd) + if !isPipe || pipe.Op != syntax.Pipe { + break + } + first = pipe.X + } + if first == guard.X { + return false + } + cat, isCall := first.Cmd.(*syntax.CallExpr) + return plainStmt(first) && isCall && len(cat.Assigns) == 0 && len(cat.Args) == 2 && cat.Args[0].Lit() == "cat" && cat.Args[1].Lit() == report +} + +// guardedCommand is the command of a statement between a status subshell's +// tests and its exit that records its own failure while the tests' status +// is still 0, and keeps the tests' status otherwise, as statusSubshell +// writes it: +// +// cat report.json | cucumber-junit > junit.xml || { converted=$?; [ "$status" -ne 0 ] || status=$converted; } +// +// The command must fall through to the exit (see fallsThrough) without +// touching either variable. saved is the variable that records its status. +// command is nil for any other statement, such as the command alone, which +// would let a passing run with a failing conversion pass when errexit is +// off. +func guardedCommand(stmt *syntax.Stmt, variable string) (command *syntax.Stmt, saved string) { + guard, isBinary := stmt.Cmd.(*syntax.BinaryCmd) + if !plainStmt(stmt) || !isBinary || guard.Op != syntax.OrStmt { + return nil, "" + } + block, isBlock := guard.Y.Cmd.(*syntax.Block) + if !plainStmt(guard.Y) || !isBlock || len(block.Stmts) != 2 { + return nil, "" + } + saved, value := onlyAssignment(block.Stmts[0]) + if saved == "" || saved == variable || !lastStatus(value) { + return nil, "" + } + if !fallsThrough(guard.X, variable) || !fallsThrough(guard.X, saved) { + return nil, "" + } + keep, isBinary := block.Stmts[1].Cmd.(*syntax.BinaryCmd) + if !plainStmt(block.Stmts[1]) || !isBinary || keep.Op != syntax.OrStmt { + return nil, "" + } + check, isCall := keep.X.Cmd.(*syntax.CallExpr) + if !plainStmt(keep.X) || !isCall || len(check.Assigns) > 0 || len(check.Args) != 5 || check.Args[0].Lit() != "[" || !readsOnly(check.Args[1], variable) || check.Args[2].Lit() != "-ne" || check.Args[3].Lit() != "0" || check.Args[4].Lit() != "]" { + return nil, "" + } + if name, value := onlyAssignment(keep.Y); name != variable || !readsOnly(value, saved) { + return nil, "" + } + return guard.X, saved +} + +// referencesVariable reports a node that expands or assigns the variable, +// as $status, ${status:-1}, status=1 or $((status + 1)) do. +func referencesVariable(node syntax.Node, variable string) bool { + found := false + syntax.Walk(node, func(node syntax.Node) bool { + switch node := node.(type) { + case *syntax.ParamExp: + found = found || (node.Param != nil && node.Param.Value == variable) + case *syntax.Assign: + found = found || (node.Name != nil && node.Name.Value == variable) + case *syntax.ArithmExp, *syntax.ArithmCmd, *syntax.LetClause: + // Arithmetic reads a variable by its bare name. + syntax.Walk(node, func(inner syntax.Node) bool { + if literal, ok := inner.(*syntax.Lit); ok && literal.Value == variable { + found = true + } + return !found + }) + } + return !found + }) + return found +} + +// statusVariables are the names of the variables statusSubshell records the +// tests' status and the conversion's in: status and converted, unless tests +// or conversion reference either (see referencesVariable), as +// cucumber-js "$status" does, which the subshell's would shadow; then +// datadog_status and datadog_converted, then those with a numeric suffix. +func statusVariables(tests, conversion string) (status, converted string) { + var nodes []syntax.Node + for _, command := range []string{tests, conversion} { + if file, ok := parseCIShell(command); ok { + nodes = append(nodes, file) + } + } + free := func(names ...string) bool { + return !slices.ContainsFunc(nodes, func(node syntax.Node) bool { + return slices.ContainsFunc(names, func(name string) bool { return referencesVariable(node, name) }) + }) + } + if free("status", "converted") { + return "status", "converted" + } + for suffix := 1; ; suffix++ { + status, converted = "datadog_status", "datadog_converted" + if suffix > 1 { + status, converted = status+"_"+strconv.Itoa(suffix), converted+"_"+strconv.Itoa(suffix) + } + if free(status, converted) { + return status, converted + } + } +} + +// plainStmt reports a statement that runs in the foreground, as written, +// without redirections. +func plainStmt(stmt *syntax.Stmt) bool { + return !stmt.Background && !stmt.Negated && !stmt.Coprocess && !stmt.Disown && len(stmt.Redirs) == 0 +} + +// onlyAssignment is the variable a plain statement that only assigns one +// sets, and the value it assigns; name is "" for any other statement. +func onlyAssignment(stmt *syntax.Stmt) (name string, value *syntax.Word) { + call, isCall := stmt.Cmd.(*syntax.CallExpr) + if !plainStmt(stmt) || !isCall || len(call.Args) > 0 || len(call.Assigns) != 1 { + return "", nil + } + assign := call.Assigns[0] + if assign.Name == nil || assign.Index != nil || assign.Array != nil || assign.Append || assign.Naked || assign.Value == nil { + return "", nil + } + return assign.Name.Value, assign.Value +} + +// lastStatus reports a word that is only $?. +func lastStatus(word *syntax.Word) bool { + if word == nil || len(word.Parts) != 1 { + return false + } + expansion, isExpansion := word.Parts[0].(*syntax.ParamExp) + return isExpansion && expansion.Short && expansion.Param != nil && expansion.Param.Value == "?" +} + +// stmtText is a statement of src as written, without the ; that ends it +// or the comments after it. +func stmtText(src string, stmt *syntax.Stmt) string { + end := stmt.End() + if stmt.Semicolon.IsValid() { + end = stmt.Semicolon + } + return strings.TrimSpace(src[stmt.Pos().Offset():end.Offset()]) +} + +// The builtins of bash and POSIX shells (bash's enable -a), which run in +// the shell itself, so can change its control flow or state, such as the +// variables a status subshell exits with: read status <<<0, printf -v +// status 0, mapfile, getopts, declare, eval, source and many more. A command +// between a status subshell's tests and its exit runs an external program +// instead, which can't, or one of statusSafeBuiltins. +var shellBuiltins = []string{".", ":", "[", "alias", "bg", "bind", "break", "builtin", "caller", "cd", "command", "compgen", "complete", "compopt", "continue", "declare", "dirs", "disown", "echo", "enable", "eval", "exec", "exit", "export", "false", "fc", "fg", "getopts", "hash", "help", "history", "jobs", "kill", "let", "local", "logout", "mapfile", "popd", "printf", "pushd", "pwd", "read", "readarray", "readonly", "return", "set", "shift", "shopt", "source", "suspend", "test", "times", "trap", "true", "type", "typeset", "ulimit", "umask", "unalias", "unset", "wait"} + +// The builtins that only print or test, and set no variable. printf is not +// one: bash's printf -v sets a variable, and so does a %n in its format. +var statusSafeBuiltins = []string{":", "[", "echo", "false", "pwd", "test", "true"} + +// Unary file tests, which read their operand as a path only. +var fileTestOperators = []string{"-e", "-f", "-s", "-d", "-r", "-w", "-x", "-L", "-h", "-p", "-S", "-b", "-c"} + +// Comparisons of integers, which bash evaluates as arithmetic, so each +// operand must be a literal integer. +var integerTestOperators = []string{"-eq", "-ne", "-lt", "-le", "-gt", "-ge"} + +// literalInteger matches a literal integer operand of test. +var literalInteger = regexp.MustCompile(`^-?[0-9]+$`) + +// plainTest reports a test or [ command, with args its arguments after the +// program, that only tests and sets nothing: a file test (see +// fileTestOperators), -n or -z on a literal, = or != between literals, or +// a comparison of literal integers (see integerTestOperators), each +// operand a literal without $, a backslash, [ or a glob character. bash +// evaluates an array subscript in other operands, as test -v +// 'a[status=0]' or test 1 -eq 'a[status=0]' do, which can assign the +// status variable; -o, -a and ! combinations aren't needed and don't +// qualify. +func plainTest(program string, args []*syntax.Word) bool { + if program == "[" { + if len(args) == 0 || shellLiteral(args[len(args)-1]) != "]" { + return false + } + args = args[:len(args)-1] + } + operands := make([]string, len(args)) + for i, arg := range args { + text, literal := shellLiteralWord(arg) + if !literal || strings.ContainsAny(text, `$\[*?`) { + return false + } + operands[i] = text + } + switch len(operands) { + case 2: + return slices.Contains(fileTestOperators, operands[0]) || operands[0] == "-n" || operands[0] == "-z" + case 3: + switch { + case operands[1] == "=" || operands[1] == "!=": + return true + case slices.Contains(integerTestOperators, operands[1]): + return literalInteger.MatchString(operands[0]) && literalInteger.MatchString(operands[2]) + } + } + return false +} + +// fallsThrough reports a command between a status subshell's tests and its +// exit that always continues to that exit with the tests' status, as a +// report conversion does: one plain command, or a pipeline of them, that +// runs in the foreground, starts with a literal program that is an +// external program or a builtin that sets nothing (see shellBuiltins; test +// and [ only with operands that set nothing, see plainTest), and neither +// reads nor assigns the status variable. Lists such as a || b, +// compound commands, functions, arithmetic and bare assignments don't +// qualify. +func fallsThrough(stmt *syntax.Stmt, variable string) bool { + if stmt.Background || stmt.Negated || stmt.Coprocess || stmt.Disown { + return false + } + switch command := stmt.Cmd.(type) { + case *syntax.BinaryCmd: + if command.Op != syntax.Pipe && command.Op != syntax.PipeAll { + return false + } + return fallsThrough(command.X, variable) && fallsThrough(command.Y, variable) + case *syntax.CallExpr: + if len(command.Args) == 0 { + return false + } + program, literal := shellLiteralWord(command.Args[0]) + if !literal || program == "" || strings.Contains(program, `\`) { + return false + } + if slices.Contains(shellBuiltins, program) && !slices.Contains(statusSafeBuiltins, program) { + return false + } + if (program == "test" || program == "[") && !plainTest(program, command.Args[1:]) { + return false + } + default: + return false + } + clean := true + syntax.Walk(stmt, func(node syntax.Node) bool { + switch node := node.(type) { + case *syntax.ParamExp: + clean = clean && (node.Param == nil || node.Param.Value != variable) + case *syntax.Assign: + clean = clean && (node.Name == nil || node.Name.Value != variable) + case *syntax.ArithmExp, *syntax.ArithmCmd, *syntax.LetClause: + clean = false + } + return clean + }) + return clean +} + +// readsOnly reports a word that is only the variable's value, as $status or +// "$status". +func readsOnly(word *syntax.Word, variable string) bool { + parts := word.Parts + if len(parts) == 1 { + if quoted, ok := parts[0].(*syntax.DblQuoted); ok { + parts = quoted.Parts + } + } + if len(parts) != 1 { + return false + } + expansion, ok := parts[0].(*syntax.ParamExp) + return ok && expansion.Param != nil && expansion.Param.Value == variable && expansion.Exp == nil && expansion.Repl == nil && expansion.Slice == nil && expansion.Index == nil && !expansion.Length && !expansion.Width && !expansion.Excl +} + // foldInlineEnvironment moves the variables a single command assigns inline // for itself into the step's environment (see foldCommandEnvironment), and // records their names: a CI command rebuilt from Run alone would drop them. @@ -114,8 +511,21 @@ func markScopedLeaf(result *commandResolution, variable, framework string) { // step's command after the scoped prefix and, for Cucumber, the // impacted-tests workaround, with cucumber-js wrapped by NYC, as onboard // --apply writes a package-script leaf. It is "" when the step already has -// that form. -func directScopedCommand(step runtimeStep, test TestCommand, framework string) (string, error) { +// that form. root is the repository, in whose test.Directory a report +// pipe's intermediate report must name no existing file (see +// cucumberReportPipe). +func directScopedCommand(root string, step runtimeStep, test TestCommand, framework string) (string, error) { + if step.statusWrapper != [2]string{} { + // The tests of a status subshell: the edit replaces them inside it, + // keeping the commands after them and the exit with their status. + tests := step + tests.statusWrapper, tests.afterTests = [2]string{}, nil + command, err := directScopedCommand(root, tests, test, framework) + if command == "" || err != nil { + return command, err + } + return step.statusWrapper[0] + command + step.statusWrapper[1], nil + } command := strings.TrimSpace(step.Run) if framework == "webdriverio" { if step.scopedVariable == scopedEnvironment(framework) { @@ -137,7 +547,28 @@ func directScopedCommand(step runtimeStep, test TestCommand, framework string) ( } command = strings.TrimSpace(strings.TrimPrefix(command, cucumberImpactedWorkaround+" ")) if err := compoundCommandError(command); err != nil { - return "", err + tests, conversion, report, ok := cucumberReportPipe(command, reportNameTaken(root, test.Directory)) + if !ok { + return "", err + } + // The tests write their report to a file, read as one simple command + // with the scoped setup; the conversion follows as its own command. + split := step + split.Run, split.scopedVariable, split.impactedWorkaround = tests, "", false + scoped, scopedErr := directScopedCommand(root, split, test, framework) + if scopedErr != nil { + return "", scopedErr + } + // ddtest prescribes only a command it reads back as these tests and + // this conversion on the next onboard --apply; for any other, such as + // a conversion through a { ...; } group, the general advice stands. + prescribed := statusSubshell(report, scoped, conversion) + read := runtimeStep{Run: prescribed} + read.readStatusSubshell() + if read.Run != scoped || !slices.Equal(read.afterTests, []string{conversion}) { + return "", err + } + return "", &ReportPipeSplit{Original: step.Run, Command: prescribed} } if !test.usesNYC { start, end, insertion, err := cucumberCoverageEdit(command) @@ -161,7 +592,150 @@ func compoundCommandError(command string) error { return nil } } - return fmt.Errorf("CI runs %s as part of a compound shell command, which ddtest does not rewrite into the scoped command: the variables its prefix assigns would reach only the first command. Run the test command as a CI command of its own, then rerun onboard --apply", command) + return fmt.Errorf("CI runs %s as part of a compound shell command, which ddtest does not rewrite into the scoped command: the variables its prefix assigns would reach only the first command. Run the test command as a CI command of its own, then rerun onboard --apply. When CI pipes the tests' output into a report converter, the tests write that report to a file instead, and the conversion of the file runs after them whatever their result: ddtest reads that conversion as outside the tests", command) +} + +// ReportPipeSplit is the rewrite of a direct Cucumber invocation that CI +// pipes into a report converter, as +// cucumber-js --format json | cucumber-junit > junit.xml: the scoped prefix +// and NYC, which prints its coverage summary to standard output, can't share +// the converter's input. The tests write the same report to a file, and the +// same CI command then converts that file whatever their result and exits +// with their status, or the conversion's when only it fails (see +// statusSubshell): failing tests still fail CI, and the report is still +// written, on providers whose scripts stop at the first failing command too. ddtest reads the conversion as outside the tests (see +// readStatusSubshell and reportTools). +type ReportPipeSplit struct { + // Original is the CI command; Command replaces it. + Original, Command string +} + +func (s *ReportPipeSplit) Error() string { + return "CI pipes the report of " + s.Original + " into a converter in one command, which ddtest does not rewrite into the scoped command. Replace 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: " + s.Command + ". Then rerun onboard --apply" +} + +// statusSubshell is the CI command that runs tests, then conversion whatever +// their result, and exits with the tests' status, or the conversion's when +// the tests pass and it fails, as the original pipe failed then too (see +// readStatusSubshell). Each status is recorded through ||, so the command +// behaves the same with errexit on or off. The conversion's own status is +// its pipe's, under whatever pipefail setting the CI shell has, as the +// original pipe's was (GitLab and CircleCI enable pipefail, Buildkite and +// Jenkins don't): without it, that is the converter's status, and the only +// earlier stage, cat, fails only on a missing report, which tests that pass +// have written. ddtest adds no set -o pipefail, which not every sh supports. +// The command first removes report, the intermediate file the tests write +// and conversion reads, so tests that fail before writing it, in a reused +// workspace or after another command that wrote it, leave cat a missing +// file, never an earlier run's report. The variables are ones neither tests +// nor conversion reference (see statusVariables). +func statusSubshell(report, tests, conversion string) string { + status, converted := statusVariables(tests, conversion) + return `(` + status + `=0; rm -f ` + report + `; ` + tests + ` || ` + status + `=$?; ` + conversion + ` || { ` + converted + `=$?; [ "$` + status + `" -ne 0 ] || ` + status + `=$` + converted + `; }; exit "$` + status + `")` +} + +// Cucumber's formatters by the extension of the file their output is +// written to. +var cucumberReportExtensions = map[string]string{"json": "json", "message": "ndjson", "junit": "xml", "html": "html"} + +// cucumberReportPipe splits a pipeline whose first command runs Cucumber +// with one formatter writing to standard output, such as +// cucumber-js ./features --format json | cucumber-junit > junit.xml, into +// that command writing the formatter's output to a file and a later command +// that pipes the file, report, into the rest of the pipeline. taken reports an +// intermediate report name that is already a file where the command runs. +// ok is false for any other command, or when the rest of the pipeline may +// start tests. +func cucumberReportPipe(command string, taken func(name string) bool) (tests, conversion, report string, ok bool) { + file, parsed := parseCIShell(command) + if !parsed || len(file.Stmts) != 1 || file.Stmts[0].Background || file.Stmts[0].Negated || file.Stmts[0].Coprocess || len(file.Stmts[0].Redirs) > 0 { + return "", "", "", false + } + var stages []*syntax.Stmt + var flatten func(*syntax.Stmt) bool + flatten = func(stmt *syntax.Stmt) bool { + if binary, ok := stmt.Cmd.(*syntax.BinaryCmd); ok { + return binary.Op == syntax.Pipe && flatten(binary.X) && flatten(binary.Y) + } + stages = append(stages, stmt) + return true + } + if !flatten(file.Stmts[0]) || len(stages) < 2 || len(stages[0].Redirs) > 0 || stages[0].Negated { + return "", "", "", false + } + call, isCall := stages[0].Cmd.(*syntax.CallExpr) + start, end := int(stages[0].Pos().Offset()), int(stages[0].End().Offset()) + if !isCall || len(call.Assigns) > 0 || len(call.Args) == 0 || !namesFramework(command[start:end], "cucumber", true) { + return "", "", "", false + } + var target *syntax.Word + value := "" + for i, arg := range call.Args[1:] { + word, literal := shellLiteralWord(arg) + option, inline, assigned := strings.Cut(word, "=") + if !literal || option != "--format" && option != "-f" { + continue + } + next := arg + if !assigned { + if i+2 >= len(call.Args) { + return "", "", "", false + } + next = call.Args[i+2] + if inline, literal = shellLiteralWord(next); !literal { + return "", "", "", false + } + } + if strings.Contains(inline, ":") { + continue // Already written to a file. + } + if target != nil { + return "", "", "", false // Two formatters share standard output. + } + target, value = next, inline + } + extension, known := cucumberReportExtensions[value] + if target == nil || !known { + return "", "", "", false + } + // The intermediate report is a file the pipeline names nowhere, so the + // conversion never writes to the file it reads, as + // cucumber-js --format junit | tee cucumber-report.xml would, and that + // is no file of the step's directory, such as a tracked fixture, which + // the tests would overwrite. + report = "cucumber-report." + extension + for suffix := 1; strings.Contains(command, report) || taken(report); suffix++ { + report = fmt.Sprintf("cucumber-report-%d.%s", suffix, extension) + } + wordStart, wordEnd := int(target.Pos().Offset()), int(target.End().Offset()) + replacement := value + ":" + report + if option, _, assigned := strings.Cut(shellLiteral(target), "="); assigned { + replacement = option + "=" + replacement + } + // The word as the shell reads it, quoted only when it needs to be. + tests = command[start:wordStart] + shellquote.Join(replacement) + command[wordEnd:end] + // The rest of the pipeline as written, without the ; or comment after + // it, which would end the prescription early. + last := stages[len(stages)-1] + rest := strings.TrimSpace(command[stages[1].Pos().Offset():last.End().Offset()]) + if mayStartTests(runtimeStep{Run: rest}, "cucumber") { + return "", "", "", false + } + return tests, "cat " + report + " | " + rest, report, true +} + +// reportNameTaken reports a name that is a file, or a path git tracks, in +// directory of the repository at root, where a CI command runs (see +// TestCommand.Directory). +func reportNameTaken(root, directory string) func(name string) bool { + return func(name string) bool { + path := filepath.Join(filepath.FromSlash(directory), name) + if _, err := os.Lstat(filepath.Join(root, path)); !errors.Is(err, fs.ErrNotExist) { + return true + } + tracked, err := exec.Command("git", "-C", root, "ls-files", "--cached", "--", filepath.ToSlash(path)).Output() + return err == nil && len(bytes.TrimSpace(tracked)) > 0 + } } // inlineAssignmentsError explains why ddtest does not rebuild a direct diff --git a/internal/onboard/ci_provider.go b/internal/onboard/ci_provider.go index ce2668bd..900ee0c5 100644 --- a/internal/onboard/ci_provider.go +++ b/internal/onboard/ci_provider.go @@ -369,6 +369,16 @@ func CIProviderTracerSetting(language, version string) string { return input + ": " + version } +// CIProviderSiteSetting names site in place of the example Datadog site the +// manual setup instructions show, which they render for the default site: +// empty for that default, which needs no substitution. +func CIProviderSiteSetting(site string) string { + if site == "" || site == defaultDatadogSite { + return "" + } + return "the Datadog site " + site + " in place of the example site " + defaultDatadogSite +} + // CIProviderAPIKeyAction is the person's step that stores the Datadog API key // for the selected CI provider. func CIProviderAPIKeyAction(skipped bool) string { diff --git a/internal/onboard/ci_provider_jenkins_pipeline.go b/internal/onboard/ci_provider_jenkins_pipeline.go index c1cf9757..0d377a48 100644 --- a/internal/onboard/ci_provider_jenkins_pipeline.go +++ b/internal/onboard/ci_provider_jenkins_pipeline.go @@ -2331,10 +2331,13 @@ func (r *jenkinsReader) add(slot *jenkinsSlot, node groovyNode, step runtimeStep } // jenkinsShellStep is where a step of a Jenkinsfile sh script starts: the -// sh call, and the statement's offset in its script. +// sh call, and the statement's offset in its script, with the quote of the +// string literal that holds the script, as the Groovy lexer read it (empty +// when the script is not a string literal). type jenkinsShellStep struct { call int statement int + quote string } // Each sh step runs a shell of its own. @@ -2355,6 +2358,10 @@ func (r *jenkinsReader) shell(call *groovyCall, scope jenkinsScope, slot *jenkin r.unresolved(slot, call, "sh has no script ddtest can read") return } + var quote string + if text, ok := node.(*groovyString); ok { + quote = text.Value.quote + } var ciOnly []string r.ciOnly = &ciOnly script, problem := r.value(node, scope) @@ -2447,7 +2454,7 @@ func (r *jenkinsReader) shell(call *groovyCall, scope jenkinsScope, slot *jenkin stmt := file.Stmts[i] known := mergeEnvironment(mergeEnvironment(env, state.env), state.references) statement := strings.TrimSpace(jenkinsStatementText(script, file.Stmts, i)) - step := runtimeStep{WorkingDirectory: state.directory, origin: jenkinsShellStep{call: call.Start, statement: int(stmt.Pos().Offset())}, sameShell: i > 0} + step := runtimeStep{WorkingDirectory: state.directory, origin: jenkinsShellStep{call: call.Start, statement: int(stmt.Pos().Offset()), quote: quote}, sameShell: i > 0} if len(state.env) > 0 { step.Env = maps.Clone(state.env) } diff --git a/internal/onboard/handwritten_setup_test.go b/internal/onboard/handwritten_setup_test.go index 1fefe201..ece0e0d3 100644 --- a/internal/onboard/handwritten_setup_test.go +++ b/internal/onboard/handwritten_setup_test.go @@ -941,7 +941,7 @@ func TestHandWrittenJenkinsDirectCucumberRoundTrip(t *testing.T) { require.ErrorAs(t, err, &gap) _, after, ok := strings.Cut(gap.Reason, "Change this CI command to exactly: ") require.True(t, ok, gap.Reason) - written, _, _ := strings.Cut(after, ". Then rerun onboard --apply") + written, _, _ := strings.Cut(after, ". This is the command as the sh step's single-quoted Groovy string") require.True(t, strings.HasPrefix(written, scopedPrefix("DD_CUCUMBER_NODE_OPTIONS")+" "+cucumberImpactedWorkaround+" "), written) writeFile(t, root, "Jenkinsfile", pipeline(written)) diff --git a/internal/onboard/instructions/java.md b/internal/onboard/instructions/java.md index 94ee7833..1589bcce 100644 --- a/internal/onboard/instructions/java.md +++ b/internal/onboard/instructions/java.md @@ -1,7 +1,7 @@ Set up Java Test Optimization for the repository's existing __FRAMEWORK__ Maven or Gradle command. 1. Keep the existing build files (POMs, Gradle settings and build scripts), Maven or Gradle wrapper, JDK versions, Maven goals or Gradle tasks, profiles, modules or projects, plugins, JVM options, test arguments, test bodies and matrices. One Maven or Gradle command runs every framework the build declares; selecting a framework does not filter the suite. Use Maven 3.2.1 or newer, or Gradle 2.0 or newer. -2. Add `datadog/test-visibility-github-action@v3` after `actions/setup-java`, before each selected Maven or Gradle test step. Set `languages: java`, `api_key: ${{ secrets.DD_API_KEY }}`, `site:` when the organization does not use `datadoghq.com`, and an exact `java-tracer-version`, such as `1.67.1`; local validation uses that release, and a floating one cannot be validated. The action adds dd-java-agent to `MAVEN_OPTS` and `GRADLE_OPTS`: a test step that sets either in its own `env` must include `${{ env.MAVEN_OPTS }}` or `${{ env.GRADLE_OPTS }}` to keep the agent, and a `java-instrumented-build-system` input must include the build tool the test steps run. In a reusable workflow (`on: workflow_call`), every caller must pass the secret: `secrets: inherit`, or `DD_API_KEY: ${{ secrets.DD_API_KEY }}` under the caller's `secrets` with `DD_API_KEY` declared under `on.workflow_call.secrets`; a required secret a caller omits fails that workflow. For JDKs the agent cannot instrument (early-access builds, Java older than 8), scope the action to the others with a matrix condition, for example `if: ${{ !contains(matrix.java, '-ea') }}`, keeping their test steps. +2. Add `datadog/test-visibility-github-action@v3` after `actions/setup-java`, before each selected Maven or Gradle test step. Set `languages: java`, `api_key: ${{ secrets.DD_API_KEY }}`, `site:` when the organization does not use `datadoghq.com`, and `java-tracer-version: __JAVA_RELEASE__`, the latest exact dd-java-agent release when ddtest printed these instructions, or another exact release; local validation uses that release, and a floating one cannot be validated. The action adds dd-java-agent to `MAVEN_OPTS` and `GRADLE_OPTS`: a test step that sets either in its own `env` must include `${{ env.MAVEN_OPTS }}` or `${{ env.GRADLE_OPTS }}` to keep the agent, and a `java-instrumented-build-system` input must include the build tool the test steps run. In a reusable workflow (`on: workflow_call`), every caller must pass the secret: `secrets: inherit`, or `DD_API_KEY: ${{ secrets.DD_API_KEY }}` under the caller's `secrets` with `DD_API_KEY` declared under `on.workflow_call.secrets`; a required secret a caller omits fails that workflow. For JDKs the agent cannot instrument (early-access builds, Java older than 8), scope the action to the others with a matrix condition, for example `if: ${{ !contains(matrix.java, '-ea') }}`, keeping their test steps. 3. Run `ddtest testdrive --framework __FRAMEWORK__ --all --yes` after the edits. It validates every discovered Maven or Gradle configuration in a disposable copy: native results (Surefire/Failsafe reports, or each Gradle test task's JUnit XML) without and with the CI-pinned agent, application coverage and the six feature controls, whose probe runs through a test task that ran. It runs only the Maven test lifecycle up to `verify`, or only Gradle test tasks, and never installs or publishes artifacts. CI rows of another JDK major are validated on the local JDK only as a substitute the user accepts with `--accept-substitute-runtime`; otherwise rerun with `JAVA_HOME` set to that JDK. `--check-only` inventories the configurations without running the build. 4. Keep `.testoptimization/report.html` and `.testoptimization/testdrive.json`, including on failure, and exclude `.testoptimization/` from Git. Post every `Open report:` link to the user. Run full validation again after edits. 5. Ask a human to create a Datadog API key and add it to the repository as the `DD_API_KEY` secret, without sharing the key itself. Local validation needs no Datadog credentials; actual CI execution and Datadog backend processing remain untested until CI runs. diff --git a/internal/onboard/instructions/java_agent.md b/internal/onboard/instructions/java_agent.md index c9d930c5..2319c30a 100644 --- a/internal/onboard/instructions/java_agent.md +++ b/internal/onboard/instructions/java_agent.md @@ -1,7 +1,7 @@ Set up Java Test Optimization in CI for the repository's existing __FRAMEWORK__ Maven or Gradle command. ddtest runs the local validation afterwards with resume_command. 1. Keep the existing build files (POMs, Gradle settings and build scripts), Maven or Gradle wrapper, JDK versions, Maven goals or Gradle tasks, profiles, modules or projects, plugins, JVM options, test arguments, test bodies and matrices. One Maven or Gradle command runs every framework the build declares; do not split it or filter it by framework. -2. Add `datadog/test-visibility-github-action@v3` after `actions/setup-java`, before each selected Maven or Gradle test step. Set `languages: java`, `api_key: ${{ secrets.DD_API_KEY }}`__SITE_INPUT__, and an exact `java-tracer-version`, such as `1.67.1`, for every instrumented JDK; never leave it empty or `latest`. The action adds dd-java-agent to `MAVEN_OPTS` and `GRADLE_OPTS`: a test step that sets either in its own `env` must include `${{ env.MAVEN_OPTS }}` or `${{ env.GRADLE_OPTS }}` to keep the agent, and a `java-instrumented-build-system` input must include the build tool the test steps run. Do not add `-javaagent` options, download the agent or set `DD_CIVISIBILITY_ENABLED` yourself. Keep CI's Maven goals and Gradle tasks, such as `install` or `publish`, unchanged: local validation runs only the Maven test lifecycle or Gradle test tasks and never installs or publishes. Tell the user that CI needs the `DD_API_KEY` secret; local validation does not. In a reusable workflow (`on: workflow_call`), every caller must pass the secret: `secrets: inherit`, or `DD_API_KEY: ${{ secrets.DD_API_KEY }}` under the caller's `secrets` with `DD_API_KEY` declared under `on.workflow_call.secrets`; a required secret a caller omits fails that workflow. +2. Add `datadog/test-visibility-github-action@v3` after `actions/setup-java`, before each selected Maven or Gradle test step. Set `languages: java`, `api_key: ${{ secrets.DD_API_KEY }}`__SITE_INPUT__, and `java-tracer-version: __JAVA_RELEASE__`, the exact dd-java-agent release ddtest selected and validates, for every instrumented JDK; never leave it empty, `latest` or another release. The action adds dd-java-agent to `MAVEN_OPTS` and `GRADLE_OPTS`: a test step that sets either in its own `env` must include `${{ env.MAVEN_OPTS }}` or `${{ env.GRADLE_OPTS }}` to keep the agent, and a `java-instrumented-build-system` input must include the build tool the test steps run. Do not add `-javaagent` options, download the agent or set `DD_CIVISIBILITY_ENABLED` yourself. Keep CI's Maven goals and Gradle tasks, such as `install` or `publish`, unchanged: local validation runs only the Maven test lifecycle or Gradle test tasks and never installs or publishes. Tell the user that CI needs the `DD_API_KEY` secret; local validation does not. In a reusable workflow (`on: workflow_call`), every caller must pass the secret: `secrets: inherit`, or `DD_API_KEY: ${{ secrets.DD_API_KEY }}` under the caller's `secrets` with `DD_API_KEY` declared under `on.workflow_call.secrets`; a required secret a caller omits fails that workflow. 3. Exclude `.testoptimization/` from Git: add `/.testoptimization/` to the repository `.gitignore`. 4. Do not run testdrive, a configuration check, Maven, Gradle or the tests yourself. Run resume_command once, after the edits above: it validates every discovered configuration and feature in disposable copies and returns the next action. When it returns setup_commands, run them as given, then its resume_command. Finish with its exact next_command and render final_response unchanged; only that coordinator result can certify `COMPLETE LOCALLY`. diff --git a/internal/onboard/java_agent_test.go b/internal/onboard/java_agent_test.go index 522e7d70..0d5c45a4 100644 --- a/internal/onboard/java_agent_test.go +++ b/internal/onboard/java_agent_test.go @@ -69,7 +69,7 @@ func TestJavaAgentOnboardingRequiresCISetupBeforeValidation(t *testing.T) { require.Contains(t, state.ResumeCommand, "datadoghq.eu") require.Contains(t, state.Reason, AgentScope) require.Contains(t, state.Instruction, "[.] ./mvnw -B verify", "the CI install step validates as verify") - for _, text := range []string{"datadog/test-visibility-github-action@v3", "languages: java", "`site: datadoghq.eu`", "exact `java-tracer-version`", "${{ env.MAVEN_OPTS }}", "/.testoptimization/", "never installs or publishes"} { + for _, text := range []string{"datadog/test-visibility-github-action@v3", "languages: java", "`site: datadoghq.eu`", "`java-tracer-version: " + testJavaRelease + "`, the exact dd-java-agent release ddtest selected", "${{ env.MAVEN_OPTS }}", "/.testoptimization/", "never installs or publishes"} { require.Contains(t, state.Instruction, text) } // The handoff runs validation once; the instructions must not add runs. diff --git a/internal/onboard/java_ci.go b/internal/onboard/java_ci.go index 22687829..233c9132 100644 --- a/internal/onboard/java_ci.go +++ b/internal/onboard/java_ci.go @@ -848,7 +848,7 @@ func JavaCIConfiguration(root, framework, site string) RuntimeCheck { case handWritten == nil && javaSetupProblem(finding, site) != "": finding.Code, finding.Reason = CodeIncompleteCISetup, javaSetupProblem(finding, site) case provider.sharedSyntax() && !exactJavaRelease.MatchString(tracer): - finding.Reason = "The Datadog action's java-tracer-version " + strconv.Quote(tracer) + " is not an exact release; set an exact version such as " + javaRecommendedTracer + "." + finding.Reason = "The Datadog action's java-tracer-version " + strconv.Quote(tracer) + " is not an exact release; set an exact version such as " + javaSetupRelease() + "." case handWritten == nil && javaReusableSecretProblem(workflows, workflow, finding.setupInputs["api_key"]) != "": finding.Code, finding.Reason = CodeIncompleteCISetup, javaReusableSecretProblem(workflows, workflow, finding.setupInputs["api_key"]) default: @@ -976,7 +976,7 @@ func javaSetupProblem(job RuntimeFinding, site string) string { inputs["java-tracer-version"] = job.TracerRequested } supported := job.setupSupported || provider.sharedSyntax() && setupActionSupported(job.Action) - return provider.setupProblem(ciSetup{Reference: job.Action, Pinned: true, Supported: supported, Problem: job.setupReadProblem, Floating: job.TracerFloating, Inputs: inputs}, "java-tracer-version", exactJavaRelease.MatchString, javaRecommendedTracer, site) + return provider.setupProblem(ciSetup{Reference: job.Action, Pinned: true, Supported: supported, Problem: job.setupReadProblem, Floating: job.TracerFloating, Inputs: inputs}, "java-tracer-version", exactJavaRelease.MatchString, javaSetupRelease(), site) } // javaAgentVariable is where the provider's Datadog setup, per its diff --git a/internal/onboard/java_handwritten.go b/internal/onboard/java_handwritten.go index be7979d6..3c4d6a47 100644 --- a/internal/onboard/java_handwritten.go +++ b/internal/onboard/java_handwritten.go @@ -29,11 +29,16 @@ import ( // javaDiscovery.validatesHandWrittenSetup), so it is never certified. // javaRecommendedTracer is the exact dd-java-agent release ddtest's setup -// instructions and messages recommend. +// names when it can't read the latest one from Maven Central (see +// javaSetupRelease): a release ddtest has validated. const javaRecommendedTracer = "1.67.1" -// javaHandWrittenDownload is an exact-release download URL for messages. -const javaHandWrittenDownload = "https://repo1.maven.org/maven2/com/datadoghq/dd-java-agent/" + javaRecommendedTracer + "/dd-java-agent-" + javaRecommendedTracer + ".jar" +// javaHandWrittenDownload is the download URL of the release ddtest selects +// (see javaSetupRelease), for messages. +func javaHandWrittenDownload() string { + release := javaSetupRelease() + return "https://repo1.maven.org/maven2/com/datadoghq/dd-java-agent/" + release + "/dd-java-agent-" + release + ".jar" +} // javaHandWrittenCarrier is a JVM option list a hand-written setup attaches // dd-java-agent through: its name, such as MAVEN_OPTS, and value. @@ -427,7 +432,7 @@ func javaHandWrittenTracer(setup *handWrittenSetup, jar string, workflow ciWorkf } switch len(named) { case 0: - setup.missing("No command of the job downloads " + jar + " with an exact release, so ddtest cannot tell which dd-java-agent it attaches: download an exact release before the tests, such as curl -Lo " + jar + " " + javaHandWrittenDownload + ".") + setup.missing("No command of the job downloads " + jar + " with an exact release, so ddtest cannot tell which dd-java-agent it attaches: download an exact release before the tests, such as curl -Lo " + jar + " " + javaHandWrittenDownload() + ".") return "" case 1: default: @@ -450,14 +455,14 @@ func javaHandWrittenTracer(setup *handWrittenSetup, jar string, workflow ciWorkf case stepDirectory(workflow, job, download) != stepDirectory(workflow, job, test): javaSimpleFormGap(setup, "downloads it in another working directory than the tests'") case strings.Contains(address, "dtdg.co/latest-java-tracer") || strings.Contains(address, "/latest/"): - setup.missing("The job downloads the latest dd-java-agent (" + address + "), so each build can attach another release: download an exact release instead, such as " + javaHandWrittenDownload + ".") + setup.missing("The job downloads the latest dd-java-agent (" + address + "), so each build can attach another release: download an exact release instead, such as " + javaHandWrittenDownload() + ".") case strings.Contains(address, "$"): setup.unresolved("the job downloads dd-java-agent from " + address + ", whose release only CI knows.") default: if release, ok := javaCanonicalRelease(address); ok { return release } - setup.missing("The job downloads dd-java-agent from " + address + ", which is not an exact release's artifact on Maven Central or the DataDog/dd-trace-java GitHub releases, so ddtest cannot tell which agent it attaches: download an exact release from one of them instead, such as " + javaHandWrittenDownload + ".") + setup.missing("The job downloads dd-java-agent from " + address + ", which is not an exact release's artifact on Maven Central or the DataDog/dd-trace-java GitHub releases, so ddtest cannot tell which agent it attaches: download an exact release from one of them instead, such as " + javaHandWrittenDownload() + ".") } return "" } diff --git a/internal/onboard/java_jenkins_test.go b/internal/onboard/java_jenkins_test.go index c8e227b6..04a6c4bc 100644 --- a/internal/onboard/java_jenkins_test.go +++ b/internal/onboard/java_jenkins_test.go @@ -79,12 +79,12 @@ func TestJavaJenkinsPlugin(t *testing.T) { // additionalVariables pin dd-java-agent by replacing the plugin's download // with an exact release's. func TestJavaJenkinsPluginPinnedRelease(t *testing.T) { - root := javaJenkinsRepository(t, javaJenkinsfile(", additionalVariables: ["+javaPluginTracerURL+": '"+javaHandWrittenDownload+"']")) + root := javaJenkinsRepository(t, javaJenkinsfile(", additionalVariables: ["+javaPluginTracerURL+": '"+javaHandWrittenDownload()+"']")) confirmJavaJenkins(t, "17") ci := JavaCIConfiguration(root, "junit5", "") require.Len(t, ci.Jobs, 1, ci.Reason) require.Equal(t, "not checked", ci.Jobs[0].Status, ci.Jobs[0].Reason) - require.Equal(t, javaRecommendedTracer, ci.Jobs[0].TracerRequested) + require.Equal(t, javaSetupRelease(), ci.Jobs[0].TracerRequested) require.False(t, ci.Jobs[0].TracerFloating) for _, url := range []string{ @@ -121,7 +121,7 @@ func TestJavaJenkinsInstructions(t *testing.T) { text := providerSetupInstructions(root, jenkins{}, "java", "junit5", "datadoghq.com", true) for _, phrase := range []string{ "languages: ['JAVA']", - javaPluginTracerURL + ": '" + javaHandWrittenDownload + "'", + javaPluginTracerURL + ": '" + javaHandWrittenDownload() + "'", "puts -javaagent before the MAVEN_OPTS and GRADLE_OPTS of every step", "--ci-runtime java=", } { @@ -180,7 +180,7 @@ func TestJavaJenkinsContainerStageApply(t *testing.T) { require.NoError(t, err) data, err := os.ReadFile(filepath.Join(root, "Jenkinsfile")) require.NoError(t, err) - require.Contains(t, string(data), `DD_SET_TRACER_VERSION_JAVA="`+javaRecommendedTracer+`"`) + require.Contains(t, string(data), `DD_SET_TRACER_VERSION_JAVA="`+javaSetupRelease()+`"`) instructions := providerSetupInstructions(root, jenkins{}, "java", "junit5", "", true) require.Contains(t, instructions, "1. Bind DD_API_KEY in those stages from a Jenkins credential") @@ -209,7 +209,7 @@ func TestJavaJenkinsShellSetupWithoutBash(t *testing.T) { require.NoError(t, err, "the image's missing Bash does not block the setup") data, err := os.ReadFile(filepath.Join(root, "Jenkinsfile")) require.NoError(t, err) - require.Contains(t, string(data), `DD_TRACER_VERSION_JAVA="`+javaRecommendedTracer+`"`) + require.Contains(t, string(data), `DD_TRACER_VERSION_JAVA="`+javaSetupRelease()+`"`) require.Contains(t, string(data), javaFallbackURL) require.Contains(t, string(data), fallbackComment[0]) require.NotContains(t, string(data), "install_test_visibility") @@ -223,7 +223,7 @@ func TestJavaJenkinsShellSetupWithoutBash(t *testing.T) { job := ci.Jobs[0] require.Empty(t, job.Code, job.Reason) require.Equal(t, fallbackSetupName, job.Action) - require.Equal(t, javaRecommendedTracer, job.TracerRequested) + require.Equal(t, javaSetupRelease(), job.TracerRequested) caveats := strings.Join(CISetupCaveats(&ci), "\n") require.Contains(t, caveats, "Jenkinsfile / Test") require.Contains(t, caveats, "With a Bash-capable image, the installer would set Test Optimization up instead, which is simpler and more reliable") @@ -238,7 +238,7 @@ func TestJavaJenkinsShellSetupWithoutBash(t *testing.T) { // and checks its SHA256; ddtest recognizes it as written, with its release // and site, and never an edited copy. func TestJavaShellSetupIsRecognizedAsWritten(t *testing.T) { - for _, tc := range []struct{ site, release string }{{"datadoghq.com", javaRecommendedTracer}, {"datadoghq.eu", "1.68.0"}} { + for _, tc := range []struct{ site, release string }{{"datadoghq.com", javaSetupRelease()}, {"datadoghq.eu", "1.68.0"}} { text := fallbackSnippet{form: jenkinsSnippetForm, language: "java", version: `"` + tc.release + `"`, site: `"` + tc.site + `"`}.shell() file, parsed := parseCIShell(text) require.True(t, parsed) @@ -248,12 +248,12 @@ func TestJavaShellSetupIsRecognizedAsWritten(t *testing.T) { require.Empty(t, setup.Problem) require.True(t, setup.fallback && setup.Supported) require.Equal(t, map[string]string{"languages": "java", "api_key": "$DD_API_KEY", "site": tc.site, "java-tracer-version": tc.release}, setup.Inputs) - require.Empty(t, fallbackSetupProblem(setup, "java-tracer-version", exactJavaRelease.MatchString, javaRecommendedTracer, tc.site)) + require.Empty(t, fallbackSetupProblem(setup, "java-tracer-version", exactJavaRelease.MatchString, javaSetupRelease(), tc.site)) } // It checks the agent's SHA256 whichever way it downloads it, runs under // set -u, where either option list may be unset, and the agent's path has // no whitespace, which the option lists split on. - written := fallbackSnippet{form: jenkinsSnippetForm, language: "java", version: `"` + javaRecommendedTracer + `"`, site: `"datadoghq.com"`}.shell() + written := fallbackSnippet{form: jenkinsSnippetForm, language: "java", version: `"` + javaSetupRelease() + `"`, site: `"datadoghq.com"`}.shell() require.Contains(t, written, `sha256sum -c -`) require.Contains(t, written, `"$DD_TRACER_FOLDER/dd-java-agent.jar.sha256" `+javaFallbackURL+`.sha256`) require.Contains(t, written, `does not match its SHA256`) @@ -280,7 +280,7 @@ func TestJavaJenkinsShellSetupResetsThePlugin(t *testing.T) { } reset := javaDiscovery{}.ciPluginReset() require.True(t, strings.HasPrefix(reset, "{ ") && strings.HasSuffix(reset, "; }"), "one compound statement") - request := ciSetupRequest{Language: "java", Site: "datadoghq.com", TracerVersion: javaRecommendedTracer} + request := ciSetupRequest{Language: "java", Site: "datadoghq.com", TracerVersion: javaSetupRelease()} fallback := newImageFallback(jenkinsContainer{reset: true}, "maven:3-alpine", "no Bash") written := fallback.snippet(request).shell() require.Contains(t, written, fallbackComment[1]+"\n"+reset+"\nif [ \"${DD_API_KEY:+set}\" = set ]; then") @@ -292,7 +292,7 @@ func TestJavaJenkinsShellSetupResetsThePlugin(t *testing.T) { problem string }{ "with the reset": {written, ""}, - "without the reset": {fallbackSnippet{form: jenkinsSnippetForm, language: "java", version: `"` + javaRecommendedTracer + `"`, site: `"datadoghq.com"`}.shell(), reset}, + "without the reset": {fallbackSnippet{form: jenkinsSnippetForm, language: "java", version: `"` + javaSetupRelease() + `"`, site: `"datadoghq.com"`}.shell(), reset}, } { t.Run(name, func(t *testing.T) { jobs := jenkinsJobs(t, scripted(tc.script+"\nmvn -B verify")) @@ -326,10 +326,10 @@ func TestJavaJenkinsShellSetupResetsThePlugin(t *testing.T) { // matches; the agent's path has no whitespace even where the workspace's // has, since MAVEN_OPTS and GRADLE_OPTS split on it. func TestJavaShellSetupDownloadsAndChecksTheAgent(t *testing.T) { - agent := "dd-java-agent " + javaRecommendedTracer + agent := "dd-java-agent " + javaSetupRelease() sum := sha256.Sum256([]byte(agent)) download := "#!/bin/sh\nout= url=\nwhile [ $# -gt 0 ]; do case \"$1\" in -o|-qO) out=\"$2\"; shift 2 ;; -*) shift ;; *) url=\"$1\"; shift ;; esac; done\n" + - "case \"$url\" in https://repo1.maven.org/maven2/com/datadoghq/dd-java-agent/" + javaRecommendedTracer + "/dd-java-agent-" + javaRecommendedTracer + ".jar) printf '%s' '" + agent + "' >\"$out\" ;; *.jar.sha256) printf '%s' \"$SUM\" >\"$out\" ;; *) exit 22 ;; esac\n" + "case \"$url\" in https://repo1.maven.org/maven2/com/datadoghq/dd-java-agent/" + javaSetupRelease() + "/dd-java-agent-" + javaSetupRelease() + ".jar) printf '%s' '" + agent + "' >\"$out\" ;; *.jar.sha256) printf '%s' \"$SUM\" >\"$out\" ;; *) exit 22 ;; esac\n" missing := "#!/bin/sh\nexit 127\n" for name, tc := range map[string]struct { curl, wget, sum string @@ -347,7 +347,7 @@ func TestJavaShellSetupDownloadsAndChecksTheAgent(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(tools, "java"), []byte(missing), 0o755)) workspace := filepath.Join(t.TempDir(), "my workspace") require.NoError(t, os.Mkdir(workspace, 0o755)) - text := fallbackSnippet{form: form, language: "java", version: `"` + javaRecommendedTracer + `"`, site: `"datadoghq.com"`}.shell() + text := fallbackSnippet{form: form, language: "java", version: `"` + javaSetupRelease() + `"`, site: `"datadoghq.com"`}.shell() shell := exec.Command("sh", "-u", "-c", text+"\nprintf '%s' \"${MAVEN_OPTS:-}|${GRADLE_OPTS:-}\"") shell.Dir = workspace shell.Env = []string{"PATH=" + tools + ":/usr/bin:/bin", "DD_API_KEY=key", "SUM=" + tc.sum, "TMPDIR=" + temporary, "MAVEN_OPTS=-Xmx1g"} @@ -473,12 +473,12 @@ func TestJavaJenkinsMixedStagesPinThePluginFirst(t *testing.T) { _, err := PlanProviderSetup(root, "junit5", "") var gap *EditThenApplyGap require.ErrorAs(t, err, &gap) - require.Contains(t, gap.Error(), "downloads the latest tracer, but this container stage's snippet installs "+javaRecommendedTracer) + require.Contains(t, gap.Error(), "downloads the latest tracer, but this container stage's snippet installs "+javaSetupRelease()) require.Contains(t, gap.Error(), "Pin the plugin to that release") // Once the host stage's plugin is pinned, the container snippet installs // the same release, and the rerun applies. - pinned := strings.Replace(mixedJenkinsfile, " agent { label 'java' }\n", " agent { label 'java' }\n options {\n datadog(testOptimization: [enabled: true, serviceName: 'app', languages: ['JAVA'], additionalVariables: ["+javaPluginTracerURL+": '"+javaHandWrittenDownload+"']])\n }\n", 1) + pinned := strings.Replace(mixedJenkinsfile, " agent { label 'java' }\n", " agent { label 'java' }\n options {\n datadog(testOptimization: [enabled: true, serviceName: 'app', languages: ['JAVA'], additionalVariables: ["+javaPluginTracerURL+": '"+javaHandWrittenDownload()+"']])\n }\n", 1) writeFile(t, root, "Jenkinsfile", pinned) plan, err := PlanProviderSetup(root, "junit5", "") require.NoError(t, err) @@ -486,7 +486,7 @@ func TestJavaJenkinsMixedStagesPinThePluginFirst(t *testing.T) { require.NoError(t, err) data, err := os.ReadFile(filepath.Join(root, "Jenkinsfile")) require.NoError(t, err) - require.Contains(t, string(data), `DD_SET_TRACER_VERSION_JAVA="`+javaRecommendedTracer+`"`, "the snippet installs the plugin's release") + require.Contains(t, string(data), `DD_SET_TRACER_VERSION_JAVA="`+javaSetupRelease()+`"`, "the snippet installs the plugin's release") } // javaJenkinsHandWrittenStage is a Jenkins stage that downloads an exact diff --git a/internal/onboard/java_language.go b/internal/onboard/java_language.go index 16a73b2c..a99fe99d 100644 --- a/internal/onboard/java_language.go +++ b/internal/onboard/java_language.go @@ -78,7 +78,7 @@ func (javaDiscovery) agentInstructions(framework, site string) string { if site != "" && site != defaultDatadogSite { siteInput = ", `site: " + site + "` (the selected Datadog site)" } - return strings.NewReplacer("__FRAMEWORK__", framework, "__SITE_INPUT__", siteInput).Replace(javaAgentInstructions) + return strings.NewReplacer("__FRAMEWORK__", framework, "__SITE_INPUT__", siteInput, "__JAVA_RELEASE__", javaSetupRelease()).Replace(javaAgentInstructions) } func (javaDiscovery) frameworks() []string { return maven.FrameworkNames } @@ -197,7 +197,7 @@ func (javaDiscovery) skipsUnsupportedNode() bool { return false } var javaInstructions string func (javaDiscovery) instructions(framework string) string { - return strings.ReplaceAll(javaInstructions, "__FRAMEWORK__", framework) + return strings.NewReplacer("__FRAMEWORK__", framework, "__JAVA_RELEASE__", javaSetupRelease()).Replace(javaInstructions) } func (javaDiscovery) onboardingScope(string) (discover, instructionsWhenConfigured bool) { @@ -252,9 +252,10 @@ func (javaDiscovery) ciSetupRequest(root, framework, site string, terms ciSetupT if terms.Plugin { return javaPluginSetupRequest(root, framework, site, terms) } - request := ciSetupRequest{Language: "java", Site: site, TracerInput: "java-tracer-version", TracerVersion: ""} + release := javaSetupRelease() + request := ciSetupRequest{Language: "java", Site: site, TracerInput: "java-tracer-version", TracerVersion: release} request.Notes = []string{ - "Replace with an exact dd-java-agent release, such as " + javaRecommendedTracer + "; local validation runs that release, and a floating one cannot be validated. Keep the existing build files, Maven or Gradle wrapper, JDK images, Maven goals or Gradle tasks, profiles, modules or projects, plugins, JVM options, test arguments and matrices. One Maven or Gradle command runs every framework the build declares: do not split it or filter it by " + framework + ". Local validation runs only the Maven test lifecycle or Gradle test tasks and never installs or publishes.", + javaReleaseNote(release) + " Keep the existing build files, Maven or Gradle wrapper, JDK images, Maven goals or Gradle tasks, profiles, modules or projects, plugins, JVM options, test arguments and matrices. One Maven or Gradle command runs every framework the build declares: do not split it or filter it by " + framework + ". Local validation runs only the Maven test lifecycle or Gradle test tasks and never installs or publishes.", javaCarriersNote(terms) + " A later command that sets one of these variables must keep the agent: extend it with " + terms.Persist(`export MAVEN_OPTS="$MAVEN_OPTS "`) + ", never replace it. Do not add -javaagent options or set DD_CIVISIBILITY_ENABLED yourself.", "ddtest reads the JDK of each instrumented job from its image, such as maven:3.9-eclipse-temurin-21, gradle:8.10-jdk17, eclipse-temurin:17-jdk or cimg/openjdk:17.0. A job that runs on the build agent's own JDK, or in an image whose name selects no JDK version, stays unverified: tell the user. Never change a job's image or JDK to make it verifiable.", setupImageNote(root, "java", framework), @@ -394,8 +395,9 @@ func (javaDiscovery) ciStepVariables(step string) map[string]string { return nil } -// dd-java-agent is no project dependency, so ddtest recommends its release. -func (javaDiscovery) recommendedTracer() string { return javaRecommendedTracer } +// dd-java-agent is no project dependency, so ddtest selects its release +// (see javaSetupRelease). +func (javaDiscovery) recommendedTracer() string { return javaSetupRelease() } // ciPluginReset removes the agent the Datadog Jenkins plugin puts before the // step's MAVEN_OPTS and GRADLE_OPTS, keeping the step's own options. diff --git a/internal/onboard/java_language_test.go b/internal/onboard/java_language_test.go index a1572a90..3fa0e7e5 100644 --- a/internal/onboard/java_language_test.go +++ b/internal/onboard/java_language_test.go @@ -29,7 +29,7 @@ func TestJavaOnboardPrintsJavaInstructions(t *testing.T) { require.True(t, strings.HasPrefix(text, "DDTest found java, junit5, and GitHub Actions.\n\nTest workflow(s):\n - .github/workflows/ci.yml\n"), text) require.Contains(t, text, " - [.] ./mvnw -B verify\n", "the discovered CI command is listed for validation") require.Contains(t, text, "lint.yml / lint: no junit5 invocation identified") - require.Contains(t, text, "an exact `java-tracer-version`", "local validation can validate only an exact release") + require.Contains(t, text, "`java-tracer-version: "+testJavaRelease+"`, the latest exact dd-java-agent release", "local validation can validate only an exact release") require.NotContains(t, text, "java-tracer-version: ''", "a floating release cannot be validated") require.Contains(t, text, "${{ env.MAVEN_OPTS }}") require.Contains(t, text, "ddtest testdrive --framework junit5 --all --yes") diff --git a/internal/onboard/java_providers.go b/internal/onboard/java_providers.go index 6356b498..82345369 100644 --- a/internal/onboard/java_providers.go +++ b/internal/onboard/java_providers.go @@ -73,7 +73,7 @@ func javaTestStep(root string, workflow ciWorkflow, job runtimeJob, step runtime func javaPluginSetupRequest(root, framework, site string, terms ciSetupTerms) ciSetupRequest { request := ciSetupRequest{Language: "java", Site: site} request.Notes = []string{ - "The " + terms.Installer + " downloads the latest dd-java-agent into the workspace (.datadog/dd-java-agent.jar, reused for 12 hours) when each build runs, unless its additionalVariables set " + javaPluginTracerURL + ". Pin an exact release, which local validation runs: add " + javaPluginTracerURL + ": '" + javaHandWrittenDownload + "' to the datadog option's additionalVariables, with the release the user chooses; the plugin then verifies the download's signature. Without it, ddtest validates locally with the latest release and reports that it floats.", + "The " + terms.Installer + " downloads the latest dd-java-agent into the workspace (.datadog/dd-java-agent.jar, reused for 12 hours) when each build runs, unless its additionalVariables set " + javaPluginTracerURL + ". Pin an exact release, which local validation runs: add " + javaPluginTracerURL + ": '" + javaHandWrittenDownload() + "' to the datadog option's additionalVariables (dd-java-agent " + javaSetupRelease() + ", the release ddtest selected: the latest on Maven Central, or --tracer-version); the plugin then verifies the download's signature. Without it, ddtest validates locally with the latest release and reports that it floats.", "The " + terms.Installer + " puts -javaagent before the MAVEN_OPTS and GRADLE_OPTS of every step, joined with a space, and sets both once per build agent: do not set MAVEN_OPTS or GRADLE_OPTS in an environment directive or withEnv; extend them inside the sh script that runs the tests (export MAVEN_OPTS=\"$MAVEN_OPTS \" before the test command). Do not add -javaagent options or set DD_CIVISIBILITY_ENABLED yourself. Keep the existing build files, wrappers, Maven goals or Gradle tasks, profiles, modules, plugins, JVM options, test arguments and matrices; one Maven or Gradle command runs every framework the build declares, so do not split it or filter it by " + framework + ". Local validation runs only the Maven test lifecycle or Gradle test tasks and never installs or publishes.", "ddtest cannot read the JDK of a Jenkins build agent: it asks the user which Java the agents run (--ci-runtime java=, or :java= per stage).", setupUnsupportedNote(root, "java", framework), diff --git a/internal/onboard/java_providers_test.go b/internal/onboard/java_providers_test.go index 3454c7ab..47ec238e 100644 --- a/internal/onboard/java_providers_test.go +++ b/internal/onboard/java_providers_test.go @@ -113,8 +113,8 @@ func TestJavaGitLabInstructions(t *testing.T) { text := providerSetupInstructions(root, gitLabCI{}, "java", "junit5", "", true) for _, phrase := range []string{ `LANGUAGES="java"`, - `JAVA_TRACER_VERSION=""`, - "Replace with an exact dd-java-agent release", + `JAVA_TRACER_VERSION="` + testJavaRelease + `"`, + "The setup installs dd-java-agent " + testJavaRelease + ", the exact release ddtest selected", "adds -javaagent for later commands of the job to MAVEN_OPTS and GRADLE_OPTS", `export MAVEN_OPTS="$MAVEN_OPTS "`, "ddtest reads the JDK of each instrumented job from its image", diff --git a/internal/onboard/java_release.go b/internal/onboard/java_release.go new file mode 100644 index 00000000..9ed20bea --- /dev/null +++ b/internal/onboard/java_release.go @@ -0,0 +1,146 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026 Datadog, Inc. + +package onboard + +import ( + "context" + "encoding/xml" + "fmt" + "io" + "net/http" + "sync" + "time" +) + +// javaAgentMetadata is Maven Central's metadata of dd-java-agent: its +// release element names the latest release. +const javaAgentMetadata = "https://repo1.maven.org/maven2/com/datadoghq/dd-java-agent/maven-metadata.xml" + +// javaReleaseResolver reads the latest exact dd-java-agent release. Tests +// replace it (see SetJavaReleaseResolver), so they never reach the network. +var javaReleaseResolver = latestJavaRelease + +// javaRelease is the dd-java-agent release this run prescribes, once +// selected or resolved. +var javaRelease struct { + sync.Mutex + value string +} + +// SetJavaReleaseResolver replaces how ddtest reads the latest dd-java-agent +// release, forgetting any release already selected, and returns the call +// that restores the previous resolver. Tests use it to stay offline. +func SetJavaReleaseResolver(resolve func(context.Context) (string, error)) (restore func()) { + javaRelease.Lock() + defer javaRelease.Unlock() + previous, selected := javaReleaseResolver, javaRelease.value + javaReleaseResolver, javaRelease.value = resolve, "" + return func() { + javaRelease.Lock() + defer javaRelease.Unlock() + javaReleaseResolver, javaRelease.value = previous, selected + } +} + +// SelectJavaRelease selects the dd-java-agent release every Java setup +// instruction, snippet and message of this run names, which validation then +// runs as the release CI pins: requested (--tracer-version) when it is an +// exact release, otherwise, for latest or empty, the latest release on Maven +// Central (see javaSetupRelease). Any other selector is an error: a floating +// release cannot be validated. +func SelectJavaRelease(requested string) (string, error) { + if requested != "" && requested != "latest" { + if !exactJavaRelease.MatchString(requested) { + return "", fmt.Errorf("the Java tracer version must be an exact dd-java-agent release, such as %s, or latest: %s", javaRecommendedTracer, requested) + } + javaRelease.Lock() + defer javaRelease.Unlock() + javaRelease.value = requested + return requested, nil + } + return javaSetupRelease(), nil +} + +// javaSetupRelease is the release SelectJavaRelease selected, or else the +// latest release on Maven Central, read once per run where a Java setup is +// first written, as the other languages resolve their tracer when they need +// it. When Maven Central can't be read, it is javaRecommendedTracer, an +// exact release ddtest has validated: the instructions still name one exact +// release, which validation then downloads. +func javaSetupRelease() string { + javaRelease.Lock() + defer javaRelease.Unlock() + if javaRelease.value == "" { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + release, err := javaReleaseResolver(ctx) + if err != nil || !exactJavaRelease.MatchString(release) { + release = javaRecommendedTracer + } + javaRelease.value = release + } + return javaRelease.value +} + +// latestJavaRelease reads the latest dd-java-agent release from Maven +// Central's metadata. +func latestJavaRelease(ctx context.Context) (string, error) { + request, err := http.NewRequestWithContext(ctx, http.MethodGet, javaAgentMetadata, nil) + if err != nil { + return "", err + } + response, err := (&http.Client{Timeout: 10 * time.Second}).Do(request) + if err != nil { + return "", err + } + defer func() { _ = response.Body.Close() }() + if response.StatusCode != http.StatusOK { + return "", fmt.Errorf("read dd-java-agent metadata: HTTP %d", response.StatusCode) + } + return readJavaRelease(io.LimitReader(response.Body, 1<<20)) +} + +// readJavaRelease reads the latest release from Maven metadata: its release +// element, or else its latest one, which must be an exact release. +func readJavaRelease(reader io.Reader) (string, error) { + var metadata struct { + Versioning struct { + Release string `xml:"release"` + Latest string `xml:"latest"` + } `xml:"versioning"` + } + if err := xml.NewDecoder(reader).Decode(&metadata); err != nil { + return "", fmt.Errorf("read dd-java-agent metadata: %w", err) + } + release := metadata.Versioning.Release + if release == "" { + release = metadata.Versioning.Latest + } + if !exactJavaRelease.MatchString(release) { + return "", fmt.Errorf("dd-java-agent metadata names no exact release: %q", release) + } + return release, nil +} + +// javaReleaseNote says, in a setup's notes, which dd-java-agent release the +// setup installs and why it is exact. +func javaReleaseNote(release string) string { + return "The setup installs dd-java-agent " + release + ", the exact release ddtest selected (the latest on Maven Central, or --tracer-version), which local validation runs: write it as shown, never a floating or other release. To set up another exact release, rerun onboard with --tracer-version ." +} + +// SelectTracerRelease applies --tracer-version (requested, "" when not +// given) to the setup of language, and returns the selector the handoff's +// commands carry. For Java, it selects the release every setup names (see +// SelectJavaRelease), and latest becomes that exact release: the setup pins +// it, so validation, resumed in another run, must compare that release with +// the CI pin, not latest. Other languages select theirs elsewhere, where +// latest stays the release their registry tags latest when it installs. +func SelectTracerRelease(language, requested string) (string, error) { + if language != "java" || requested == "" { + return requested, nil + } + return SelectJavaRelease(requested) +} diff --git a/internal/onboard/java_release_test.go b/internal/onboard/java_release_test.go new file mode 100644 index 00000000..8fa4fb9d --- /dev/null +++ b/internal/onboard/java_release_test.go @@ -0,0 +1,182 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026 Datadog, Inc. + +package onboard + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "os" + "path/filepath" + "slices" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// useJavaRelease makes release the latest dd-java-agent release for one +// test, counting how often ddtest reads it. +func useJavaRelease(t *testing.T, release string, err error) *int { + t.Helper() + reads := 0 + t.Cleanup(SetJavaReleaseResolver(func(context.Context) (string, error) { + reads++ + return release, err + })) + return &reads +} + +// Maven Central's metadata names the latest release in its release element, +// or else in its latest one; anything else is not an exact release. +func TestReadJavaReleaseFromMavenMetadata(t *testing.T) { + for metadata, want := range map[string]string{ + "com.datadoghq1.68.11.68.11.67.11.68.1": "1.68.1", + "1.69.0": "1.69.0", + "1.70.0-SNAPSHOT": "", + "": "", + "": "", + } { + release, err := readJavaRelease(strings.NewReader(metadata)) + require.Equal(t, want, release, metadata) + require.Equal(t, want == "", err != nil, metadata) + } +} + +// The latest release is read once per run; an exact --tracer-version wins +// without reading it; a floating selector is refused; and an unreadable +// Maven Central falls back to the release ddtest has validated. +func TestSelectJavaRelease(t *testing.T) { + reads := useJavaRelease(t, "1.70.2", nil) + require.Equal(t, "1.70.2", javaSetupRelease()) + release, err := SelectJavaRelease("latest") + require.NoError(t, err) + require.Equal(t, "1.70.2", release) + require.Equal(t, 1, *reads) + + reads = useJavaRelease(t, "1.70.2", nil) + release, err = SelectJavaRelease("1.66.0") + require.NoError(t, err) + require.Equal(t, "1.66.0", release) + require.Equal(t, "1.66.0", javaSetupRelease()) + require.Zero(t, *reads) + + for _, floating := range []string{"1.66", "1.x", "git:main", "1.68.1-SNAPSHOT"} { + _, err = SelectJavaRelease(floating) + require.ErrorContains(t, err, "exact dd-java-agent release", floating) + } + + useJavaRelease(t, "", errors.New("offline")) + require.Equal(t, javaRecommendedTracer, javaSetupRelease()) + useJavaRelease(t, "latest", nil) + require.Equal(t, javaRecommendedTracer, javaSetupRelease()) +} + +// Every Java setup ddtest prescribes names the release it selected, never a +// placeholder or one the agent picks: the GitLab script, the CircleCI orb, +// the CircleCI setup without Bash, the Buildkite snippet, the GitHub action +// instructions and the hand-written download it suggests. +func TestJavaSetupsNameTheSelectedRelease(t *testing.T) { + useJavaRelease(t, "1.70.2", nil) + for name, tc := range map[string]struct { + provider ciProvider + file string + config string + want []string + }{ + "gitlab": {gitLabCI{}, gitLabCIFile, "test:\n image: maven:3.9-eclipse-temurin-21\n script:\n - mvn -B verify\n", []string{`JAVA_TRACER_VERSION="1.70.2"`}}, + "circleci": {circleCI{}, circleCIFile, "version: 2.1\njobs:\n test:\n docker:\n - image: cimg/openjdk:17.0\n steps:\n - checkout\n - run: mvn -B verify\n" + circleWorkflow(" - test\n"), []string{"java_tracer_version: 1.70.2"}}, + "circleci, no bash": {circleCI{}, circleCIFile, "version: 2.1\njobs:\n test:\n docker:\n - image: eclipse-temurin:17-jdk-alpine\n steps:\n - checkout\n - run: mvn -B verify\n" + circleWorkflow(" - test\n"), []string{`DD_TRACER_VERSION_JAVA="1.70.2"`}}, + "buildkite": {buildkite{}, buildkiteFile, "steps:\n - label: test\n command: mvn -B verify\n plugins:\n - docker#v5.12.0:\n image: maven:3.9-eclipse-temurin-17\n", []string{`DD_SET_TRACER_VERSION_JAVA="1.70.2"`}}, + } { + t.Run(name, func(t *testing.T) { + root := t.TempDir() + writeFile(t, root, "pom.xml", javaJUnit5POM) + writeFile(t, root, tc.file, tc.config) + t.Chdir(root) + useCIProvider(t, tc.provider.name()) + text := providerSetupInstructions(root, tc.provider, "java", "junit5", "", true) + for _, want := range append(tc.want, "The setup installs dd-java-agent 1.70.2, the exact release ddtest selected") { + require.Contains(t, text, want) + } + require.NotContains(t, text, "") + require.NotContains(t, text, javaRecommendedTracer) + }) + } + github := javaDiscovery{}.agentInstructions("junit5", "") + require.Contains(t, github, "`java-tracer-version: 1.70.2`") + require.Contains(t, javaDiscovery{}.instructions("junit5"), "`java-tracer-version: 1.70.2`") + require.Equal(t, "https://repo1.maven.org/maven2/com/datadoghq/dd-java-agent/1.70.2/dd-java-agent-1.70.2.jar", javaHandWrittenDownload()) +} + +// onboard --agent for Java prints the setup with the release +// --tracer-version names, and refuses a floating one before any setup. +func TestJavaOnboardingHonorsTracerVersion(t *testing.T) { + useJavaRelease(t, "1.70.2", nil) + root := gitLabJavaRepository(t, "test:\n image: maven:3.9-eclipse-temurin-21\n script:\n - mvn -B verify\n") + writeFile(t, root, ".gitignore", "/.testoptimization/\n") + var output bytes.Buffer + _ = StartWithOptions(&output, StartOptions{Format: "json", Tracer: "1.66.0"}) + require.Contains(t, output.String(), `JAVA_TRACER_VERSION=\"1.66.0\"`, output.String()) + require.NotContains(t, output.String(), "1.70.2") + + useJavaRelease(t, "1.70.2", nil) + output.Reset() + _ = StartWithOptions(&output, StartOptions{Format: "json", Tracer: "1.66"}) + require.Contains(t, output.String(), `"code":"invalid_options"`) + require.Contains(t, output.String(), "exact dd-java-agent release") +} + +// An explicit --tracer-version latest selects the exact latest release, as +// no --tracer-version does: the setup pins it, and every command of the +// handoff carries that release, never latest, so the resumed validation +// compares it with the CI pin. +func TestJavaOnboardingResolvesAnExplicitLatest(t *testing.T) { + useJavaRelease(t, "1.70.2", nil) + gitLabJavaRepository(t, "test:\n image: maven:3.9-eclipse-temurin-21\n script:\n - mvn -B verify\n") + var output bytes.Buffer + _ = StartWithOptions(&output, StartOptions{Format: "json", Tracer: "latest"}) + var result AgentResult + require.NoError(t, json.Unmarshal(output.Bytes(), &result), output.String()) + require.Contains(t, output.String(), `JAVA_TRACER_VERSION=\"1.70.2\"`, output.String()) + commands := [][]string{result.NextCommand, result.ResumeCommand, result.AcceptResumeCommand} + require.NotEmpty(t, slices.Concat(commands...), output.String()) + for _, command := range commands { + if i := slices.Index(command, "--tracer-version"); i >= 0 { + require.Equal(t, "1.70.2", command[i+1], "%v", command) + } + } + require.Contains(t, slices.Concat(commands...), "1.70.2", "the handoff carries the exact release") + require.NotContains(t, slices.Concat(commands...), "latest") + + selected, err := SelectTracerRelease("java", "latest") + require.NoError(t, err) + require.Equal(t, "1.70.2", selected) + for _, language := range []string{"javascript", "python", "ruby"} { + selected, err := SelectTracerRelease(language, "latest") + require.NoError(t, err) + require.Equal(t, "latest", selected, "%s installs the release its registry tags latest", language) + } +} + +// The Jenkins container snippet installs the release the host stages' plugin +// pins (see jenkinsPluginRelease), even when ddtest selected another. +func TestJavaJenkinsSnippetFollowsThePluginPin(t *testing.T) { + useJavaRelease(t, "1.70.2", nil) + root := javaJenkinsRepository(t, mixedJenkinsfile) + pin := "https://repo1.maven.org/maven2/com/datadoghq/dd-java-agent/1.66.0/dd-java-agent-1.66.0.jar" + pinned := strings.Replace(mixedJenkinsfile, " agent { label 'java' }\n", " agent { label 'java' }\n options {\n datadog(testOptimization: [enabled: true, serviceName: 'app', languages: ['JAVA'], additionalVariables: ["+javaPluginTracerURL+": '"+pin+"']])\n }\n", 1) + writeFile(t, root, "Jenkinsfile", pinned) + plan, err := PlanProviderSetup(root, "junit5", "") + require.NoError(t, err) + _, err = plan.Apply(true) + require.NoError(t, err) + data, err := os.ReadFile(filepath.Join(root, "Jenkinsfile")) + require.NoError(t, err) + require.Contains(t, string(data), `DD_SET_TRACER_VERSION_JAVA="1.66.0"`) + require.NotContains(t, string(data), "1.70.2") +} diff --git a/internal/onboard/java_scripts.go b/internal/onboard/java_scripts.go index 98ba2575..b1633288 100644 --- a/internal/onboard/java_scripts.go +++ b/internal/onboard/java_scripts.go @@ -22,7 +22,7 @@ import ( // Any other executable, such as the JVM, another build tool, a task runner, // a package manager, a command runner (xargs, timeout, ssh) or one ddtest // doesn't know, may. -var javaNonLaunchingTools = slices.Concat(javaReleaseTools, checkAndPackageTools, []string{ +var javaNonLaunchingTools = slices.Concat(javaReleaseTools, checkAndPackageTools, reportTools, []string{ "read", "local", "declare", "typeset", "readonly", "unset", "shift", "return", "false", ":", "trap", "wait", "umask", "getopts", "rmdir", "chown", "stat", "mktemp", "readlink", "realpath", "uname", "whoami", "hostname", "nproc", "id", "printenv", "envsubst", "yq", "groupadd", "useradd", "usermod", "groupmod", "addgroup", "adduser", "update-ca-certificates", diff --git a/internal/onboard/main_test.go b/internal/onboard/main_test.go new file mode 100644 index 00000000..31b34e10 --- /dev/null +++ b/internal/onboard/main_test.go @@ -0,0 +1,23 @@ +// Unless explicitly stated otherwise all files in this repository are licensed +// under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2026 Datadog, Inc. + +package onboard + +import ( + "context" + "os" + "testing" +) + +// testJavaRelease is the latest dd-java-agent release tests read: Java +// setups name it instead of the offline fallback, javaRecommendedTracer. +const testJavaRelease = "1.68.1" + +// Tests never read Maven Central: the latest dd-java-agent release is +// testJavaRelease. +func TestMain(m *testing.M) { + SetJavaReleaseResolver(func(context.Context) (string, error) { return testJavaRelease, nil }) + os.Exit(m.Run()) +} diff --git a/internal/onboard/non_test_steps_test.go b/internal/onboard/non_test_steps_test.go index a1ea8228..c3b31c9a 100644 --- a/internal/onboard/non_test_steps_test.go +++ b/internal/onboard/non_test_steps_test.go @@ -877,3 +877,42 @@ func TestVueCLIPluginCommandsAreUnreadPackages(t *testing.T) { require.Contains(t, unresolved, "/e2e") require.Contains(t, unresolved, "docs_then_tests / step 1 (npm run styleguide:build)") } + +// Report converters read the results a test run left and start no test +// runner, in any language's CI: a conversion after the tests is outside +// them. A scoped package of the same name, or an unknown converter, may. +func TestReportConvertersCannotStartTests(t *testing.T) { + root := t.TempDir() + writeFile(t, root, "package.json", `{"scripts":{"test":"cucumber-js"}}`) + for run, want := range map[string]bool{ + "cat report.json | ./node_modules/.bin/cucumber-junit > junit.xml": false, + "npx cucumber-junit@1.7.1 < report.json > junit.xml": false, + "npx junit-merge -d reports -o junit.xml": false, + "npx jrm junit.xml 'reports/*.xml'": false, + "npx mochawesome-merge reports/*.json > report.json && npx marge report.json": false, + "npx xunit-viewer -r reports -o report.html": false, + "allure generate allure-results --clean": false, + "tail -n 20 report.json | tee summary.txt": false, + "npx @acme/cucumber-junit < report.json": true, + "cat report.json | ./node_modules/.bin/report-tool > junit.xml": true, + } { + require.Equal(t, want, mayStartTestsIn(root, root, runtimeStep{Run: run}, "cucumber", 0), run) + } + require.False(t, javaShellMayRunTests(root, root, "cat target/report.json | cucumber-junit > junit.xml\njunit2html junit.xml report.html\n", nil, false, 0)) + require.True(t, javaShellMayRunTests(root, root, "cat target/report.json | report-tool > junit.xml\n", nil, false, 0)) +} + +// A GitLab job that converts the Cucumber report in a later script line, as +// onboarding prescribes for a report pipe, keeps that line outside the +// tests: it is a caveat, not unresolved scope. +func TestPostTestReportConversionIsOutsideTheTests(t *testing.T) { + root := t.TempDir() + writeFile(t, root, "package.json", `{"devDependencies":{"@cucumber/cucumber":"11.0.1","nyc":"17.1.0","cucumber-junit":"1.7.1"}}`) + writeFile(t, root, gitLabCIFile, "acceptance:\n image: node:24\n script:\n - npm install\n - ./node_modules/.bin/cucumber-js ./features --format json:report.json\n - cat report.json | ./node_modules/.bin/cucumber-junit > junit.xml\n") + useCIProvider(t, gitLabCIProvider) + scope, err := DiscoverValidationScope(root, "cucumber") + require.NoError(t, err) + require.Empty(t, scope.Unresolved) + require.Len(t, scope.Commands, 1) + require.Contains(t, strings.Join(scope.Caveats, "\n"), "step 3 (cat report.json | ./node_modules/.bin/cucumber-junit > junit.xml) runs after the job's last cucumber test and can't start cucumber") +} diff --git a/internal/onboard/non_test_tools.go b/internal/onboard/non_test_tools.go index 8b7d27a0..11434036 100644 --- a/internal/onboard/non_test_tools.go +++ b/internal/onboard/non_test_tools.go @@ -26,6 +26,17 @@ var checkAndPackageTools = []string{ "shellcheck", "hadolint", "actionlint", "yamllint", "editorconfig-checker", "typos", "codespell", } +// Test-report converters, mergers and viewers: they read result files a test +// run left (or stdin) and write a report, and start no test runner. A CI step +// that converts the tests' report after them, as +// cat cucumber.json | cucumber-junit > junit.xml, is outside the tests. +// Converters run as a test framework's reporter option, such as +// mocha-junit-reporter, are not commands and are not listed. +var reportTools = []string{ + "cucumber-junit", "cucumber-junit-convert", "junit-merge", "junit-report-merger", "jrm", + "mochawesome-merge", "marge", "mochawesome-report-generator", "xunit-viewer", "junit-viewer", "allure", "junit2html", +} + // Publishing CLIs that only upload what the project built, running at most // the package's own publish lifecycle scripts. Tools documented to run the // project's tests, a build command or other hooks, such as np, release-it, @@ -60,7 +71,7 @@ func recognizedTool(root, directory string, words []string, framework string, de name := strings.ToLower(filepath.Base(words[0])) args := words[1:] switch { - case slices.Contains(checkAndPackageTools, name): + case slices.Contains(checkAndPackageTools, name) || reportTool(words[0]): return false, true case name == "docker" || name == "podman": return dockerMayStart(root, directory, args, depth, func(body string) bool { @@ -83,6 +94,13 @@ func recognizedTool(root, directory string, words []string, framework string, de return false, false } +// reportTool reports a known report converter (see reportTools) by its +// executable or unscoped package name, pinned or not: a scoped package of the +// same name, such as @acme/cucumber-junit, has no documented behavior. +func reportTool(executable string) bool { + return !strings.HasPrefix(executable, "@") && slices.Contains(reportTools, publishToolName(executable)) +} + // publishTool reports a known publish-only CLI by its executable or package // name. A package merely named publish, release or deploy, such as // @acme/deploy, has no documented behavior and may start the tests itself, diff --git a/internal/onboard/provider_scoped_setup_test.go b/internal/onboard/provider_scoped_setup_test.go index ab749a58..0a4f5da0 100644 --- a/internal/onboard/provider_scoped_setup_test.go +++ b/internal/onboard/provider_scoped_setup_test.go @@ -9,11 +9,16 @@ import ( "context" "encoding/json" "errors" + "fmt" + "io/fs" "os" + "os/exec" "path/filepath" + "strconv" "strings" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "mvdan.cc/sh/v3/syntax" ) @@ -52,6 +57,17 @@ var scopedProviders = []scopedProvider{ }, } +// scopedProvidersNamed is the scoped provider of name, for a test whose +// behavior doesn't depend on the provider. +func scopedProvidersNamed(name string) []scopedProvider { + for _, provider := range scopedProviders { + if provider.name == name { + return []scopedProvider{provider} + } + } + panic("no scoped provider " + name) +} + // scopedRepository is a repository whose only CI is provider's config, // selected for the test, with the given package.json and files. func scopedRepository(t *testing.T, provider scopedProvider, config string, pkg map[string]any, files map[string]string) string { @@ -671,3 +687,1158 @@ func TestProviderDirectWebdriverIOReadsItsDirectorysConfig(t *testing.T) { require.Error(t, err) require.False(t, errors.As(err, &gap), "%v", err) } + +// A direct Cucumber invocation that CI pipes into a report converter gets +// the exact replacement: the tests write the same report to a file with the +// scoped setup, and the same command converts the file whatever their result +// and exits with their status, or the conversion's when only it fails. +// onboard --apply then accepts it, reading the conversion as outside the +// tests. +func TestProviderSplitsACucumberReportPipe(t *testing.T) { + for _, provider := range scopedProviders { + t.Run(provider.name, func(t *testing.T) { + framework := scopedFrameworks[0] + pipe := "./node_modules/.bin/cucumber-js ./features --format json | ./node_modules/.bin/cucumber-junit > junit.xml" + root := scopedRepository(t, provider, provider.config("npm ci", pipe), framework.pkg, nil) + _, err := PlanJavaScriptSetup(root, "cucumber", "5.133.0", "datadoghq.com", ">=18") + var gap *EditThenApplyGap + require.ErrorAs(t, err, &gap) + _, after, ok := strings.Cut(gap.Reason, "or the conversion's when only the conversion fails: ") + require.True(t, ok, gap.Reason) + command, _, ok := strings.Cut(after, ". Then rerun onboard --apply") + require.True(t, ok, gap.Reason) + tests := cucumberScopedOptions + " " + cucumberImpactedWorkaround + " npx --no-install nyc -- ./node_modules/.bin/cucumber-js ./features --format json:cucumber-report.json" + conversion := "cat cucumber-report.json | ./node_modules/.bin/cucumber-junit > junit.xml" + want := `(status=0; rm -f cucumber-report.json; ` + tests + ` || status=$?; ` + conversion + ` || { converted=$?; [ "$status" -ne 0 ] || status=$converted; }; exit "$status")` + if provider.name == buildkiteProvider { + want = buildkiteEscape(want) // As the pipeline holds it. + } + require.Equal(t, want, command) + + writeFile(t, root, provider.file, provider.config("npm ci", command, "echo later")) + plan, err := PlanJavaScriptSetup(root, "cucumber", "5.133.0", "datadoghq.com", ">=18") + require.NoError(t, err) + require.Contains(t, plannedFile(t, plan, provider.file), command) + scope, err := DiscoverValidationScope(root, "cucumber") + require.NoError(t, err) + require.Empty(t, scope.Unresolved) + require.Len(t, scope.Commands, 1) + require.Equal(t, "nyc -- ./node_modules/.bin/cucumber-js ./features --format json:cucumber-report.json", scope.Commands[0].Command, "the tests are the configuration") + require.Contains(t, scope.Commands[0].StepScopeNotes, "This CI step runs "+conversion+" 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.") + }) + } +} + +// The status subshell converts the report whatever the tests' result, and +// exits with the tests' status, or the conversion's when only the +// conversion fails, as the original pipe did then: with errexit on, as CI +// scripts run, and off, and with pipefail for the conversion's own pipe. +// Only a passing run continues with the script's later commands under +// errexit. sh runs every combination; bash, also with pipefail, and dash run +// those where the tests, the conversion or both fail, but for the +// three-stage conversion, which only sh needs to show the last stage's +// status counts. +func TestStatusSubshellConvertsAndKeepsTheTestsStatus(t *testing.T) { + shells := realShells(t, "sh", "bash", "dash") + tests := map[string]int{"(echo report > report.json; exit 3)": 3, "echo report > report.json": 0} + conversions := map[string]int{ + "cat report.json | tr a-z A-Z > junit.xml": 0, + "cat report.json | sh -c 'tr a-z A-Z > junit.xml; exit 5'": 5, + "cat report.json | tr a-z A-Z | sh -c 'cat > junit.xml; exit 6'": 6, + } + for _, shell := range shells { + options := []string{"set -e", "set +e"} + if shell == "bash" { + options = append(options, "set -e -o pipefail") + } + var cases []shellCase + var labels []string + var wants []int + var errexit []bool + for _, option := range options { + for tests, testStatus := range tests { + for conversion, conversionStatus := range conversions { + if shell != "sh" && (conversionStatus == 6 || testStatus == 0 && conversionStatus == 0) { + continue + } + want := testStatus + if want == 0 { + want = conversionStatus + } + cases = append(cases, shellCase{script: option + "\n" + statusSubshell("report.json", tests, conversion) + "\nstatus=$?\necho later > later.txt\nexit \"$status\"\n"}) + labels = append(labels, shell+" "+option+" "+tests+" "+conversion) + wants = append(wants, want) + errexit = append(errexit, strings.HasPrefix(option, "set -e")) + } + } + } + for i, result := range runShellCases(t, shell, cases) { + label, want := labels[i], wants[i] + assert.Equal(t, want, result.status, "the tests' status, or the conversion's when only it fails: %s", label) + converted, readErr := os.ReadFile(filepath.Join(result.dir, "junit.xml")) + if assert.NoError(t, readErr, "the report is converted whatever the tests' result: %s", label) { + assert.Equal(t, "REPORT\n", string(converted), label) + } + _, later := os.Stat(filepath.Join(result.dir, "later.txt")) + if errexit[i] { + assert.Equal(t, want == 0, later == nil, "only a passing run continues with the script: %s", label) + } else { + assert.NoError(t, later, "without errexit, the script continues: %s", label) + } + } + } +} + +// realShells is those of names on PATH; the test skips without any. +func realShells(t *testing.T, names ...string) []string { + t.Helper() + var shells []string + for _, name := range names { + if _, err := exec.LookPath(name); err == nil { + shells = append(shells, name) + } + } + if len(shells) == 0 { + t.Skip("no sh") + } + return shells +} + +// shellCase is a script a real shell runs in a directory of its own, which +// holds files first. +type shellCase struct { + script string + files map[string]string +} + +// shellResult is the status a case's script exited with and the directory +// it ran in. +type shellResult struct { + status int + dir string +} + +// runShellCases runs each case's script in a fresh process of shell, in the +// case's own directory, all from one process of shell that records every +// status: a matrix of cases costs the test one process, not one per case. +func runShellCases(t *testing.T, shell string, cases []shellCase) []shellResult { + t.Helper() + root := t.TempDir() + results := make([]shellResult, len(cases)) + for i, c := range cases { + name := fmt.Sprintf("%03d", i) + results[i].dir = filepath.Join(root, name) + require.NoError(t, os.Mkdir(results[i].dir, 0o755)) + for path, contents := range c.files { + require.NoError(t, os.WriteFile(filepath.Join(results[i].dir, path), []byte(contents), 0o644)) + } + require.NoError(t, os.WriteFile(filepath.Join(root, name+".sh"), []byte(c.script), 0o644)) + } + driver := `for script in *.sh; do name=${script%.sh}; (cd "$name" && exec "$1" "../$script"); echo "$?" > "$name.status"; done` + command := exec.Command(shell, "-c", driver, "_", shell) + command.Dir = root + output, err := command.CombinedOutput() + require.NoError(t, err, "%s", output) + for i := range results { + data, err := os.ReadFile(filepath.Join(root, fmt.Sprintf("%03d.status", i))) + require.NoError(t, err, "%s", output) + results[i].status, err = strconv.Atoi(strings.TrimSpace(string(data))) + require.NoError(t, err) + } + return results +} + +// The status subshell removes the intermediate report before the tests, so +// tests that fail before writing it, in a reused workspace or after another +// command that wrote it, never get an earlier run's report converted: cat +// finds no file, and the command exits with the tests' status whatever the +// converter does with no input. sh runs it with errexit on and off, bash +// with pipefail, where cat's failure is the conversion's status, and dash +// with errexit. +func TestStatusSubshellNeverConvertsAStaleReport(t *testing.T) { + conversions := []string{ + "cat report.json | tr a-z A-Z > junit.xml", + `cat report.json | sh -c 'input=$(cat); [ -n "$input" ] || exit 7; echo "$input" > junit.xml'`, + } + for _, shell := range realShells(t, "sh", "bash", "dash") { + options := map[string][]string{"sh": {"set -e", "set +e"}, "bash": {"set -e -o pipefail", "set +e -o pipefail"}, "dash": {"set -e"}}[shell] + var cases []shellCase + var labels []string + for _, option := range options { + for _, conversion := range conversions { + cases = append(cases, shellCase{script: option + "\n" + statusSubshell("report.json", "sh -c 'exit 4'", conversion) + "\n", files: map[string]string{"report.json": "stale\n"}}) + labels = append(labels, shell+" "+option+" "+conversion) + } + } + for i, result := range runShellCases(t, shell, cases) { + label := labels[i] + assert.Equal(t, 4, result.status, "the tests' status: %s", label) + _, statErr := os.Stat(filepath.Join(result.dir, "report.json")) + assert.ErrorIs(t, statErr, fs.ErrNotExist, "the stale report is removed: %s", label) + converted, readErr := os.ReadFile(filepath.Join(result.dir, "junit.xml")) + if strings.Contains(label, "exit 7") { + assert.ErrorIs(t, readErr, fs.ErrNotExist, "the converter got no input: %s", label) + } else if assert.NoError(t, readErr, "the conversion ran after the failing tests: %s", label) { + assert.Empty(t, string(converted), "no earlier run's report is converted: %s", label) + } + } + } +} + +// Every provider whose scripts stop at the first failing command reads only +// the prescribed removal before a status subshell's tests: any other, or +// one of another file, leaves the command unresolved. +func TestEveryProviderReadsOnlyThePrescribedRemoval(t *testing.T) { + tests := "./node_modules/.bin/cucumber-js ./features --format json:cucumber-report.json" + conversion := "cat cucumber-report.json | ./node_modules/.bin/cucumber-junit > junit.xml" + guard := ` || { converted=$?; [ "$status" -ne 0 ] || status=$converted; }; exit "$status")` + for name, write := range statusSubshellProviders { + t.Run(name, func(t *testing.T) { + root := t.TempDir() + writeFile(t, root, "package.json", `{"devDependencies":{"@cucumber/cucumber":"11.0.1","cucumber-junit":"1.7.1"}}`) + t.Chdir(root) + useCIProvider(t, name) + write(t, root, "npm ci", statusSubshell("cucumber-report.json", tests, conversion)) + scope, err := DiscoverValidationScope(root, "cucumber") + require.NoError(t, err) + require.Empty(t, scope.Unresolved) + require.Len(t, scope.Commands, 1) + require.Equal(t, "cucumber-js ./features --format json:cucumber-report.json", scope.Commands[0].Command) + + // TestStatusSubshellReadsOnlyThePrescribedForm rejects every other + // removal at the reader; each provider reaches it with one of + // another file and one that Buildkite escapes. + for _, removal := range []string{"rm -f other.json", "rm -f $REPORT"} { + write(t, root, "npm ci", `(status=0; `+removal+`; `+tests+` || status=$?; `+conversion+guard) + scope, err := DiscoverValidationScope(root, "cucumber") + require.NoError(t, err) + require.Empty(t, scope.Commands, removal) + require.NotEmpty(t, scope.Unresolved, removal) + } + }) + } +} + +// Every status subshell ddtest prescribes for a report pipe is one ddtest +// reads back as the same tests and conversion on the next onboard --apply, +// and one the shell parses: a pipe whose conversion the reader can't read, +// such as one through a { ...; } group or one that reads the status +// variable, keeps the general advice instead, and a ; or comment after the +// pipe stays out of the prescription. +func TestEveryReportPipePrescriptionReadsBack(t *testing.T) { + root := t.TempDir() + var prescriptions []string + for pipe, prescribed := range map[string]bool{ + "cucumber-js --format json | cucumber-junit > junit.xml": true, + "cucumber-js --format json | cucumber-junit > junit.xml;": true, + "cucumber-js --format json | cucumber-junit > junit.xml ; ": true, + "cucumber-js --format json | cucumber-junit > junit.xml # convert": true, + "cucumber-js --format json | cucumber-junit > junit.xml; # convert": true, + "cucumber-js --format json |\n cucumber-junit > junit.xml": true, + "npx cucumber-js './features/a b' --format \"json\" | cucumber-junit --name 'My suite' > \"out dir/junit.xml\"": true, + "cucumber-js --format json | tee raw.json | cucumber-junit > junit.xml": true, + "cucumber-js --format json | cucumber-junit 2>&1 > junit.xml": true, + "cucumber-js --format json | FORCE_COLOR=0 cucumber-junit > junit.xml": true, + "cucumber-js --format=message | npx @cucumber/junit-xml-formatter > junit.xml": false, + "cucumber-js --format json | { cucumber-junit; }": false, + "cucumber-js --format json | { cucumber-junit > junit.xml; } # convert": false, + "cucumber-js --format json | (cucumber-junit > junit.xml)": false, + // The subshell's variables are ones the pipe doesn't reference. + "cucumber-js --format json | cucumber-junit > \"$status.xml\"": true, + "cucumber-js --format json | cucumber-junit --name \"$converted\"": true, + "cucumber-js \"$status\" --format json | cucumber-junit": true, + "cucumber-js \"$converted\" --format json | cucumber-junit": true, + "cucumber-js ${status:-a} --format json | cucumber-junit --name \"$datadog_converted\"": true, + "cucumber-js --format json | cucumber-junit | exit 0": false, + "CI=1 cucumber-js --format json | cucumber-junit": false, + "cucumber-js --format json | cucumber-junit > junit.xml; echo done": false, + "cucumber-js --format json | cucumber-junit > junit.xml && echo done": false, + } { + t.Run(pipe, func(t *testing.T) { + _, err := directScopedCommand(root, runtimeStep{Run: pipe}, TestCommand{}, "cucumber") + require.Error(t, err) + var split *ReportPipeSplit + if !prescribed { + require.False(t, errors.As(err, &split), "%v", err) + require.Contains(t, err.Error(), "as part of a compound shell command") + return + } + require.ErrorAs(t, err, &split) + read := runtimeStep{Run: split.Command} + read.readStatusSubshell() + require.Len(t, read.afterTests, 1, split.Command) + require.Contains(t, read.Run, "cucumber-js", split.Command) + require.Equal(t, split.Command, read.statusWrapper[0]+read.Run+read.statusWrapper[1]) + original, ok := parseCIShell(pipe) + require.True(t, ok) + status, converted := statusVariables(read.Run, read.afterTests[0]) + require.True(t, strings.HasPrefix(split.Command, "("+status+"=0; "), split.Command) + require.Contains(t, split.Command, "{ "+converted+"=$?; ") + require.False(t, referencesVariable(original, status), "the pipe's $%s is not the subshell's", status) + require.False(t, referencesVariable(original, converted), "the pipe's $%s is not the subshell's", converted) + require.NotContains(t, split.Command, "#") + require.NotContains(t, split.Command, ";;") + prescriptions = append(prescriptions, split.Command) + }) + } + requireShellsParse(t, prescriptions, "bash", "dash") +} + +// requireShellsParse checks that each shell of names on PATH parses every +// command, with one shell -n over a script holding each command as the +// body of a function of its own, where an unclosed quote or group fails the +// script. +func requireShellsParse(t *testing.T, commands []string, names ...string) { + t.Helper() + var script strings.Builder + for i, command := range commands { + fmt.Fprintf(&script, "prescription_%d() {\n%s\n}\n", i, command) + } + path := filepath.Join(t.TempDir(), "prescriptions.sh") + require.NoError(t, os.WriteFile(path, []byte(script.String()), 0o644)) + for _, shell := range names { + if _, err := exec.LookPath(shell); err != nil { + continue + } + output, err := exec.Command(shell, "-n", path).CombinedOutput() + require.NoError(t, err, "%s -n:\n%s\n%s", shell, script.String(), output) + } +} + +// Only the exact status-preserving subshell is read as tests followed by +// commands outside them; a command after the tests that may start them, a +// missing status or another exit stays unresolved. +func TestStatusSubshellReadsOnlyThePrescribedForm(t *testing.T) { + guard := ` || { converted=$?; [ "$status" -ne 0 ] || status=$converted; }` + for run, want := range map[string][2]any{ + `(status=0; npx cucumber-js --format json:r.json || status=$?; cat r.json | cucumber-junit > j.xml` + guard + `; exit "$status")`: {"npx cucumber-js --format json:r.json", []string{"cat r.json | cucumber-junit > j.xml"}}, + "(\n rc=0\n npx cucumber-js || rc=$?\n npx junit-merge -d r -o j.xml || { saved=$?; [ \"$rc\" -ne 0 ] || rc=$saved; }\n allure generate r || { saved=$?; [ $rc -ne 0 ] || rc=\"$saved\"; }\n exit $rc\n)": {"npx cucumber-js", []string{"npx junit-merge -d r -o j.xml", "allure generate r"}}, + // The prescription removes the report the tests write and the conversion reads first. + `(status=0; rm -f r.json; npx cucumber-js --format json:r.json || status=$?; cat r.json | cucumber-junit > j.xml` + guard + `; exit "$status")`: {"npx cucumber-js --format json:r.json", []string{"cat r.json | cucumber-junit > j.xml"}}, + `(status=0; rm -f r.json; npx cucumber-js --format=json:r.json || status=$?; cat r.json | cucumber-junit` + guard + `; npx allure generate r` + guard + `; exit "$status")`: {"npx cucumber-js --format=json:r.json", []string{"cat r.json | cucumber-junit", "npx allure generate r"}}, + // Any other removal, or one of another file, stays unresolved. + `(status=0; rm -f other.json; npx cucumber-js --format json:r.json || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; rm -f r.json; npx cucumber-js --format json:other.json || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; rm -f r.json; npx cucumber-js --format json:r.json || status=$?; cat other.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; rm -f r.json; npx cucumber-js --format json:r.json || status=$?; cucumber-junit < r.json` + guard + `; exit "$status")`: {}, + `(status=0; rm -f r.json; npx cucumber-js --format json:r.json || status=$?; cat r.json` + guard + `; exit "$status")`: {}, + `(status=0; rm -f r.json; npx cucumber-js --format json:r.json || status=$?; exit "$status")`: {}, + `(status=0; rm -rf r.json; npx cucumber-js --format json:r.json || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; rm -rf .; npx cucumber-js --format json:r.json || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; rm r.json; npx cucumber-js --format json:r.json || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; rm -f r.json x; npx cucumber-js --format json:r.json || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; rm -f -- r.json; npx cucumber-js --format json:r.json || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; rm -f *.json; npx cucumber-js --format json:*.json || status=$?; cat *.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; rm -f "$r"; npx cucumber-js --format json:$r || status=$?; cat "$r" | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; rm -f ../r.json; npx cucumber-js --format json:../r.json || status=$?; cat ../r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; rm -f /r.json; npx cucumber-js --format json:/r.json || status=$?; cat /r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; rm -f r.json > log; npx cucumber-js --format json:r.json || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; rm -f r.json & npx cucumber-js --format json:r.json || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; F=1 rm -f r.json; npx cucumber-js --format json:r.json || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(rm -f r.json; status=0; npx cucumber-js --format json:r.json || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; rm -f r.json; rm -f r.json; npx cucumber-js --format json:r.json || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + // The round-2 form, without the guard, would let a failing conversion pass without errexit. + `(status=0; npx cucumber-js || status=$?; cat r.json | cucumber-junit > j.xml; exit "$status")`: {}, + `(status=0; npx cucumber-js || status=$?; cat r.json | cucumber-junit` + guard + `; exit 0)`: {}, + `(status=1; npx cucumber-js || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; npx cucumber-js || other=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; npx cucumber-js && status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; npx cucumber-js || status=$?; exit "$status")`: {}, + `(status=0; npx cucumber-js || status=$?; cat r.json | cucumber-junit` + guard + `; exit "${status:-1}")`: {}, + `(status=0; npx cucumber-js || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status") > log`: {}, + `status=0; npx cucumber-js || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status"`: {}, + // Guards that lose the tests' status, or the conversion's. + `(status=0; npx cucumber-js || status=$?; cucumber-junit || { converted=$?; status=$converted; }; exit "$status")`: {}, + `(status=0; npx cucumber-js || status=$?; cucumber-junit || { converted=$?; [ "$status" -eq 0 ] && status=$converted; }; exit "$status")`: {}, + `(status=0; npx cucumber-js || status=$?; cucumber-junit || { converted=$?; [ "$status" -ne 1 ] || status=$converted; }; exit "$status")`: {}, + `(status=0; npx cucumber-js || status=$?; cucumber-junit || { converted=$?; [ "$other" -ne 0 ] || status=$converted; }; exit "$status")`: {}, + `(status=0; npx cucumber-js || status=$?; cucumber-junit || { converted=$?; [ "$status" -ne 0 ] || status=$other; }; exit "$status")`: {}, + `(status=0; npx cucumber-js || status=$?; cucumber-junit || { converted=1; [ "$status" -ne 0 ] || status=$converted; }; exit "$status")`: {}, + `(status=0; npx cucumber-js || status=$?; cucumber-junit || { status=$?; [ "$status" -ne 0 ] || status=$status; }; exit "$status")`: {}, + `(status=0; npx cucumber-js || status=$?; cucumber-junit || { converted=$?; [ "$status" -ne 0 ] || status=$converted; exit 0; }; exit "$status")`: {}, + `(status=0; npx cucumber-js || status=$?; cucumber-junit || ( converted=$?; [ "$status" -ne 0 ] || status=$converted ); exit "$status")`: {}, + `(status=0; npx cucumber-js || status=$?; cucumber-junit $converted || { converted=$?; [ "$status" -ne 0 ] || status=$converted; }; exit "$status")`: {}, + `(status=0; npx cucumber-js || status=$?; exit 0` + guard + `; exit "$status")`: {}, + // Tests that read either variable would get the subshell's value, not CI's. + `(status=0; npx cucumber-js "$status" || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; npx cucumber-js ${status:-1} || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; npx cucumber-js $((status + 1)) || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + `(status=0; npx cucumber-js --name "$converted" || status=$?; cat r.json | cucumber-junit` + guard + `; exit "$status")`: {}, + // Other variable names, as ddtest picks when the pipe references status or converted. + `(datadog_status=0; rm -f r.json; npx cucumber-js "$status" --format json:r.json || datadog_status=$?; cat r.json | cucumber-junit --name "$converted" || { datadog_converted=$?; [ "$datadog_status" -ne 0 ] || datadog_status=$datadog_converted; }; exit "$datadog_status")`: {"npx cucumber-js \"$status\" --format json:r.json", []string{"cat r.json | cucumber-junit --name \"$converted\""}}, + `(datadog_status=0; npx cucumber-js "$datadog_status" || datadog_status=$?; cat r.json | cucumber-junit || { datadog_converted=$?; [ "$datadog_status" -ne 0 ] || datadog_status=$datadog_converted; }; exit "$datadog_status")`: {}, + `(datadog_status=0; npx cucumber-js "$datadog_converted" || datadog_status=$?; cat r.json | cucumber-junit || { datadog_converted=$?; [ "$datadog_status" -ne 0 ] || datadog_status=$datadog_converted; }; exit "$datadog_status")`: {}, + } { + step := runtimeStep{Run: run} + step.readStatusSubshell() + if want[0] == nil { + require.Equal(t, run, step.Run, run) + require.Empty(t, step.afterTests, run) + continue + } + require.Equal(t, want[0], step.Run, run) + require.Equal(t, want[1], step.afterTests, run) + } + root := t.TempDir() + writeFile(t, root, "package.json", `{"devDependencies":{"@cucumber/cucumber":"11.0.1"},"scripts":{"report":"cucumber-js"}}`) + writeFile(t, root, gitLabCIFile, "acceptance:\n image: node:24\n script:\n - npm install\n - (status=0; npx cucumber-js || status=$?; npm run report || { converted=$?; [ \"$status\" -ne 0 ] || status=$converted; }; exit \"$status\")\n") + useCIProvider(t, gitLabCIProvider) + scope, err := DiscoverValidationScope(root, "cucumber") + require.NoError(t, err) + require.Contains(t, strings.Join(scope.Unresolved, "\n"), "CI runs npm run report after the tests in the same command, and it may start cucumber itself") +} + +// Only a pipe from one Cucumber formatter on standard output into commands +// that can't start tests is split; any other compound command keeps the +// general advice. +func TestCucumberReportPipeSplitsOnlyAReportConversion(t *testing.T) { + for command, want := range map[string][2]string{ + "cucumber-js --format json | cucumber-junit > junit.xml": {"cucumber-js --format json:cucumber-report.json", "cat cucumber-report.json | cucumber-junit > junit.xml"}, + "npx cucumber-js -f message | npx @cucumber/junit-xml-formatter": {}, + "cucumber-js --format=junit | tee junit.xml": {"cucumber-js --format=junit:cucumber-report.xml", "cat cucumber-report.xml | tee junit.xml"}, + "cucumber-js --format json --format html:report.html | cucumber-junit": {"cucumber-js --format json:cucumber-report.json --format html:report.html", "cat cucumber-report.json | cucumber-junit"}, + "cucumber-js --format json --format html | cucumber-junit": {}, + "cucumber-js --format json | node convert.js": {}, + "cucumber-js --format json:out.json | cucumber-junit": {}, + "cucumber-js --format pretty | cucumber-junit": {}, + "cucumber-js --format json |& cucumber-junit": {}, + "npm run build && cucumber-js --format json | cucumber-junit": {}, + "cat x.json | cucumber-junit": {}, + } { + tests, conversion, report, ok := cucumberReportPipe(command, noFile) + require.Equal(t, want[0] != "", ok, command) + if ok { + require.Equal(t, "cat "+report+" | ", conversion[:len("cat "+report+" | ")], command) + } + require.Equal(t, want, [2]string{tests, conversion}, command) + } +} + +// Every provider whose scripts stop at the first failing command reads the +// status-preserving subshell the same way: its tests are the configuration, +// its conversion a note outside the tests, and the commands after it still +// run in the same shell. +func TestEveryProviderReadsTheStatusSubshell(t *testing.T) { + tests := "./node_modules/.bin/cucumber-js ./features --format json:cucumber-report.json" + conversion := "cat cucumber-report.json | ./node_modules/.bin/cucumber-junit > junit.xml" + command := statusSubshell("cucumber-report.json", tests, conversion) + require.Contains(t, command, "(status=0; rm -f cucumber-report.json; ./node_modules/.bin/cucumber-js") + jenkinsfile := "pipeline {\n agent any\n stages {\n stage('Test') {\n steps {\n sh 'npm ci'\n sh '''\n " + command + "\n npx eslint .\n '''\n }\n }\n }\n}\n" + for name, write := range map[string]func(root string){ + gitLabCIProvider: func(root string) { + writeFile(t, root, gitLabCIFile, "test:\n image: node:22\n script:\n - npm ci\n - "+command+"\n - npx eslint .\n") + }, + circleCIProvider: func(root string) { + writeFile(t, root, circleCIFile, "version: 2.1\njobs:\n test:\n docker:\n - image: cimg/node:22.11\n steps:\n - checkout\n - run: npm ci\n - run: "+command+"\n - run: npx eslint .\n"+circleWorkflow(" - test\n")) + }, + buildkiteProvider: func(root string) { + writeFile(t, root, buildkiteFile, "steps:\n - label: test\n command:\n - npm ci\n - "+buildkiteEscape(command)+"\n - npx eslint .\n plugins:\n - docker#v5.12.0:\n image: node:22\n") + }, + jenkinsProvider: func(root string) { writeFile(t, root, "Jenkinsfile", jenkinsfile) }, + } { + t.Run(name, func(t *testing.T) { + root := t.TempDir() + writeFile(t, root, "package.json", `{"devDependencies":{"@cucumber/cucumber":"11.0.1","cucumber-junit":"1.7.1","eslint":"9.0.0"}}`) + write(root) + t.Chdir(root) + useCIProvider(t, name) + scope, err := DiscoverValidationScope(root, "cucumber") + require.NoError(t, err) + require.Empty(t, scope.Unresolved) + require.Len(t, scope.Commands, 1) + require.Equal(t, "cucumber-js ./features --format json:cucumber-report.json", scope.Commands[0].Command, "the tests are the configuration") + require.Contains(t, scope.Commands[0].StepScopeNotes, "This CI step runs "+conversion+" 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.") + }) + } +} + +// statusSubshellProviders writes a job whose script runs commands in order, +// in each provider whose scripts stop at the first failing command, as its +// configuration holds them (Buildkite's with $$). +var statusSubshellProviders = map[string]func(t *testing.T, root string, commands ...string){ + gitLabCIProvider: func(t *testing.T, root string, commands ...string) { + writeFile(t, root, gitLabCIFile, "test:\n image: node:22\n script:\n - "+strings.Join(commands, "\n - ")+"\n") + }, + circleCIProvider: func(t *testing.T, root string, commands ...string) { + writeFile(t, root, circleCIFile, "version: 2.1\njobs:\n test:\n docker:\n - image: cimg/node:22.11\n steps:\n - checkout\n - run: "+strings.Join(commands, "\n - run: ")+"\n"+circleWorkflow(" - test\n")) + }, + buildkiteProvider: func(t *testing.T, root string, commands ...string) { + escaped := make([]string, len(commands)) + for i, command := range commands { + escaped[i] = buildkiteEscape(command) + } + writeFile(t, root, buildkiteFile, "steps:\n - label: test\n command:\n - "+strings.Join(escaped, "\n - ")+"\n plugins:\n - docker#v5.12.0:\n image: node:22\n") + }, + jenkinsProvider: func(t *testing.T, root string, commands ...string) { + writeFile(t, root, "Jenkinsfile", "pipeline {\n agent any\n stages {\n stage('Test') {\n steps {\n sh '''\n "+strings.Join(commands, "\n ")+"\n '''\n }\n }\n }\n}\n") + }, +} + +// A command between a status subshell's tests and its exit that can change +// the shell's flow, or the status it exits with, makes the subshell +// unresolved: the saved-status exit might never run, as after exit 0. +func TestStatusSubshellRejectsCommandsThatSkipTheExit(t *testing.T) { + for _, middle := range append([]string{ + "exit 0", "return 0", "exec cucumber-junit", "break", "continue", "set +e", `\exit 0`, "'exit' 0", "eval exit 0", + "source ./convert.sh", ". ./convert.sh", "trap 'exit 0' EXIT", "builtin exit 0", "command exit 0", + "cucumber-junit < r.json & :", "cat r.json | cucumber-junit || true", "cucumber-junit < r.json && exit 0", + "status=0", "status=0 cucumber-junit", "export status=0", "unset status", ": $((status=0))", "let status=0", "echo ${status:=0}", + "(exit 0)", "{ exit 0; }", "if true; then exit 0; fi", "f() { exit 0; }", "! cucumber-junit", "$CONVERT r.json", + }, statusUnsafeBuiltins...) { + t.Run(middle, func(t *testing.T) { + run := `(status=0; npx cucumber-js || status=$?; ` + middle + ` || { converted=$?; [ "$status" -ne 0 ] || status=$converted; }; exit "$status")` + step := runtimeStep{Run: run} + step.readStatusSubshell() + require.Equal(t, run, step.Run) + require.Empty(t, step.afterTests) + }) + } + for _, middle := range []string{"cat r.json | cucumber-junit > j.xml", "cucumber-junit < r.json > j.xml 2>&1", "status-report r.json", "echo $other", ": done", "test -f j.xml", "sort -o status r.txt", "/usr/bin/printf -v", "/usr/bin/printf '%n' x"} { + step := runtimeStep{Run: `(status=0; npx cucumber-js || status=$?; ` + middle + ` || { converted=$?; [ "$status" -ne 0 ] || status=$converted; }; exit "$status")`} + step.readStatusSubshell() + require.Equal(t, []string{middle}, step.afterTests, middle) + } + for name, write := range statusSubshellProviders { + t.Run(name, func(t *testing.T) { + root := t.TempDir() + writeFile(t, root, "package.json", `{"devDependencies":{"@cucumber/cucumber":"11.0.1"}}`) + write(t, root, "npm ci", `(status=0; ./node_modules/.bin/cucumber-js || status=$?; exit 0; exit "$status")`) + t.Chdir(root) + useCIProvider(t, name) + scope, err := DiscoverValidationScope(root, "cucumber") + require.NoError(t, err) + require.Empty(t, scope.Commands, "an exit 0 before the saved status masks failing tests") + require.NotEmpty(t, scope.Unresolved) + + // The reader rejects every unsafe builtin above; each provider + // reaches it with one that quotes, and one that Buildkite escapes. + for _, middle := range []string{"printf '%n' status", "[[ -n $((status=0)) ]]"} { + require.Contains(t, statusUnsafeBuiltins, middle) + write(t, root, "npm ci", `(status=0; ./node_modules/.bin/cucumber-js || status=$?; `+middle+` || { converted=$?; [ "$status" -ne 0 ] || status=$converted; }; exit "$status")`) + scope, err := DiscoverValidationScope(root, "cucumber") + require.NoError(t, err) + require.Empty(t, scope.Commands, "%s overwrites the saved status", middle) + require.NotEmpty(t, scope.Unresolved, middle) + } + }) + } +} + +// Builtins that write a shell variable named by an argument, here the saved +// status, so the subshell's exit would mask failing tests, or change how the +// shell runs later commands; no builtin but those that only print or test +// may run after the tests. bash's printf writes one through -v or a %n in +// its format, so no printf builtin qualifies, even one that only prints; +// /usr/bin/printf, a separate process, can't write the shell's variables. +var statusUnsafeBuiltins = []string{ + "printf '%n' status", "printf 'x'", "printf -v status 0", "printf -vstatus 0", "printf -- -v", `printf "$format" 0`, "read status <<<0", "read -r status < r.txt", + "mapfile status < r.txt", "readarray -t status < r.txt", "getopts a status", "wait -p status", "compgen -V status -W 0", + "hash -p /bin/true cucumber-junit", "[[ -n $((status=0)) ]]", +} + +// statusSubshellAfter is a status subshell whose tests are followed by +// middle, with the guard that keeps their status. +func statusSubshellAfter(tests, middle string) string { + return `(status=0; ` + tests + ` || status=$?; ` + middle + ` || { converted=$?; [ "$status" -ne 0 ] || status=$converted; }; exit "$status")` +} + +// test and [ after the tests qualify only with operands bash never +// evaluates as arithmetic: a file test, -n or -z on a literal, = or != +// between literals, or a comparison of literal integers. Any other operand +// set can assign the saved status through an array subscript, as test -v +// 'BASH_VERSINFO[status=0]' does, or is a combination the reader doesn't +// need, so the subshell stays unresolved. The guard's own [ "$status" -ne +// 0 ] is a separate shape that keeps being read. +var ( + statusUnsafeTests = []string{ + "test -v 'BASH_VERSINFO[status=0]'", "[ -v 'a[status=0]' ]", "test -f 'x[status=0]'", `[ -n "$x" ]`, + "test 1 -eq 'a[status=0]'", "[ 'a[status=0]' -ne 0 ]", "test -v status", "test -f j.xml -o -v 'a[status=0]'", "[ -f j.xml -a -s j.xml ]", + "[ ! -f j.xml ]", "test j.xml", "test", "[ ]", "[ -f j.xml", "test -f j.xml ]", "test -f '$x'", `test -f x\[1\]`, "test -f j*.xml", "test -t 1", + "test a == b", "test 1x -eq 1", "[[ -f j.xml ]]", + } + statusSafeTests = []string{ + "test -f junit.xml", "[ -s report.xml ]", "test -e j.xml", "[ -d reports ]", "test -L j.xml", "test -n x", "[ -z '' ]", + "test a = b", `[ "a b" != c ]`, "test 1 -eq 1", "[ -1 -lt 2 ]", "test 0 -ge 0", + } +) + +func TestStatusSubshellAcceptsOnlyPlainTests(t *testing.T) { + for _, middle := range statusUnsafeTests { + run := statusSubshellAfter("npx cucumber-js", middle) + step := runtimeStep{Run: run} + step.readStatusSubshell() + require.Equal(t, run, step.Run, middle) + require.Empty(t, step.afterTests, middle) + } + for _, middle := range statusSafeTests { + step := runtimeStep{Run: statusSubshellAfter("npx cucumber-js", middle)} + step.readStatusSubshell() + require.Equal(t, "npx cucumber-js", step.Run, middle) + require.Equal(t, []string{middle}, step.afterTests, middle) + } + // The guard's own test still reads, after a file test. + step := runtimeStep{Run: `(status=0; npx cucumber-js || status=$?; [ -s j.xml ] || { converted=$?; [ "$status" -ne 0 ] || status=$converted; }; cat j.xml || { converted=$?; [ $status -ne 0 ] || status="$converted"; }; exit "$status")`} + step.readStatusSubshell() + require.Equal(t, []string{"[ -s j.xml ]", "cat j.xml"}, step.afterTests) + + // Every provider reaches the reader's operand check: two rejected + // forms, one per program, and one accepted. + for name, write := range statusSubshellProviders { + t.Run(name, func(t *testing.T) { + root := t.TempDir() + writeFile(t, root, "package.json", `{"devDependencies":{"@cucumber/cucumber":"11.0.1"}}`) + t.Chdir(root) + useCIProvider(t, name) + for _, middle := range []string{"test -v 'BASH_VERSINFO[status=0]'", "[ -v 'a[status=0]' ]"} { + require.Contains(t, statusUnsafeTests, middle) + write(t, root, "npm ci", statusSubshellAfter("./node_modules/.bin/cucumber-js", middle)) + scope, err := DiscoverValidationScope(root, "cucumber") + require.NoError(t, err) + require.Empty(t, scope.Commands, "%s overwrites the saved status", middle) + require.NotEmpty(t, scope.Unresolved, middle) + } + write(t, root, "npm ci", statusSubshellAfter("./node_modules/.bin/cucumber-js", "test -f junit.xml")) + scope, err := DiscoverValidationScope(root, "cucumber") + require.NoError(t, err) + require.Empty(t, scope.Unresolved) + require.Len(t, scope.Commands, 1) + require.Contains(t, scope.Commands[0].StepScopeNotes, "This CI step runs test -f junit.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.") + }) + } +} + +// In a real bash, test -v 'BASH_VERSINFO[status=0]' after failing tests +// resets the saved status through its subscript, so the subshell exits 0, +// with errexit and pipefail on, which is why the reader rejects it; a file +// test, which it accepts, keeps the tests' status. +func TestStatusSubshellTestVariableMasksFailingTestsInBash(t *testing.T) { + realShells(t, "bash") + if exec.Command("bash", "-c", "x=1; test -v x").Run() != nil { + t.Skip("this bash has no test -v") + } + masking := statusSubshellAfter("(exit 3)", "test -v 'BASH_VERSINFO[status=0]'") + plain := statusSubshellAfter("(exit 3)", "test -f junit.xml") + results := runShellCases(t, "bash", []shellCase{ + {script: "set -e -o pipefail\n" + masking + "\n"}, + {script: "set -e -o pipefail\n" + plain + "\n", files: map[string]string{"junit.xml": ""}}, + }) + require.Equal(t, 0, results[0].status, "the failing tests' status is masked") + require.Equal(t, 3, results[1].status, "a file test keeps the tests' status") + for middle, read := range map[string]bool{"test -v 'BASH_VERSINFO[status=0]'": false, "test -f junit.xml": true} { + step := runtimeStep{Run: statusSubshellAfter("npx cucumber-js", middle)} + step.readStatusSubshell() + require.Equal(t, read, len(step.afterTests) > 0, middle) + } +} + +// The commands after a status subshell's tests are read as the same +// commands after the tests in a plain sequence: a file change stays setup +// for a later test step, and a command ddtest can't resolve is outside the +// tests only until a later test follows it. +func TestStatusSubshellTailsPrepareLaterTests(t *testing.T) { + for name, write := range statusSubshellProviders { + t.Run(name, func(t *testing.T) { + root := t.TempDir() + writeFile(t, root, "package.json", `{"devDependencies":{"@cucumber/cucumber":"11.0.1","cucumber-junit":"1.7.1"}}`) + write(t, root, "npm ci", `(status=0; ./node_modules/.bin/cucumber-js features/a || status=$?; cp ci.env .env || { converted=$?; [ "$status" -ne 0 ] || status=$converted; }; exit "$status")`, "./node_modules/.bin/cucumber-js features/b") + t.Chdir(root) + useCIProvider(t, name) + scope, err := DiscoverValidationScope(root, "cucumber") + require.NoError(t, err) + require.Contains(t, strings.Join(scope.Unresolved, "\n"), "CI file or dependency changes require separate validation: cp ci.env .env", "the later tests read the file it changes") + + write(t, root, "npm ci", `(status=0; ./node_modules/.bin/cucumber-js features/a || status=$?; ./node_modules/.bin/cucumber-junit < r.json > j.xml || { converted=$?; [ "$status" -ne 0 ] || status=$converted; }; exit "$status")`, "./node_modules/.bin/cucumber-js features/b") + scope, err = DiscoverValidationScope(root, "cucumber") + require.NoError(t, err) + require.Contains(t, strings.Join(scope.Unresolved, "\n"), "but before a later cucumber test step, which it may prepare") + }) + } +} + +// An existing status subshell whose tests still need the scoped prefix and +// NYC gets the whole subshell back with only its tests edited, so following +// the prescription keeps the conversion and the exit with the tests' status; +// onboard --apply then accepts it. +func TestProviderEditsTheTestsInsideAStatusSubshell(t *testing.T) { + for _, provider := range scopedProviders { + t.Run(provider.name, func(t *testing.T) { + framework := scopedFrameworks[0] + conversion := "cat cucumber-report.json | ./node_modules/.bin/cucumber-junit > junit.xml" + original := statusSubshell("cucumber-report.json", "./node_modules/.bin/cucumber-js ./features --format json:cucumber-report.json", conversion) + escape := func(command string) string { return command } + if provider.name == buildkiteProvider { + escape = buildkiteEscape // As the pipeline holds it. + } + root := scopedRepository(t, provider, provider.config("npm ci", escape(original)), framework.pkg, nil) + _, err := PlanJavaScriptSetup(root, "cucumber", "5.133.0", "datadoghq.com", ">=18") + var gap *EditThenApplyGap + require.ErrorAs(t, err, &gap) + _, after, ok := strings.Cut(gap.Reason, "Change this CI command to exactly: ") + require.True(t, ok, gap.Reason) + command, _, ok := strings.Cut(after, ". Then rerun onboard --apply") + require.True(t, ok, gap.Reason) + want := statusSubshell("cucumber-report.json", cucumberScopedOptions+" "+cucumberImpactedWorkaround+" npx --no-install nyc -- ./node_modules/.bin/cucumber-js ./features --format json:cucumber-report.json", conversion) + require.Equal(t, escape(want), command) + + writeFile(t, root, provider.file, provider.config("npm ci", command)) + plan, err := PlanJavaScriptSetup(root, "cucumber", "5.133.0", "datadoghq.com", ">=18") + require.NoError(t, err) + require.Contains(t, plannedFile(t, plan, provider.file), command) + }) + } +} + +// jenkinsContainerTestStage is a Jenkinsfile whose Test stage runs in a +// container, where onboard --apply sets the tracer up with the setup +// snippet, with npm ci and then script, an sh step's argument as written. +func jenkinsContainerTestStage(script string) string { + return "pipeline {\n agent { docker { image 'node:22' } }\n stages {\n stage('Test') {\n steps {\n sh 'npm ci'\n sh " + script + "\n }\n }\n }\n}\n" +} + +// jenkinsWritten is the shell text of the sh step a Jenkinsfile runs last, +// as Groovy reads it without its surrounding blank space, with each ${…} +// it interpolates written back as ${…}, and the expressions it +// interpolates. +func jenkinsWritten(t *testing.T, jenkinsfile string) (string, []string) { + t.Helper() + tokens, err := lexGroovy(jenkinsfile) + require.NoError(t, err) + var text string + var interpolated []string + for _, token := range tokens { + if token.str == nil { + continue + } + var written strings.Builder + interpolated = nil + for _, part := range token.str.parts { + if part.interpolated { + written.WriteString("${" + part.expr + "}") + interpolated = append(interpolated, part.expr) + continue + } + written.WriteString(part.text) + } + text = strings.TrimSpace(written.String()) + } + return text, interpolated +} + +// jenkinsLiterals are the four string literals an sh step's script can be +// written in, each holding a script whose first command reads SUITE, which +// the pipeline's environment sets (see jenkinsSuiteStage), as Groovy (in a +// GString) or the shell (in a single-quoted string) expands it, then a line +// break or ; and the shell command it is given, escaped as that literal +// needs; with what Groovy hands the shell before that command and the +// expressions it interpolates. +var jenkinsLiterals = []struct { + name, first string + interpolated []string + script func(command string) string +}{ + {"double-quoted", "echo ${env.SUITE}; ", []string{"env.SUITE"}, func(command string) string { + return `"echo ${env.SUITE}; ` + strings.NewReplacer(`\`, `\\`, `"`, `\"`, "$", `\$`).Replace(command) + `"` + }}, + {"triple-double-quoted", "echo ${env.SUITE}\n ", []string{"env.SUITE"}, func(command string) string { + return "\"\"\"\n echo ${env.SUITE}\n " + strings.NewReplacer(`\`, `\\`, "$", `\$`).Replace(command) + "\n \"\"\"" + }}, + {"single-quoted", `echo "$SUITE"; `, nil, func(command string) string { + return `'echo "$SUITE"; ` + strings.NewReplacer(`\`, `\\`, "'", `\'`).Replace(command) + "'" + }}, + {"triple-single-quoted", "echo \"$SUITE\"\n ", nil, func(command string) string { + return "'''\n echo \"$SUITE\"\n " + strings.ReplaceAll(command, `\`, `\\`) + "\n '''" + }}, +} + +// jenkinsSuiteStage is jenkinsContainerTestStage with SUITE set in the +// pipeline's environment, which ddtest resolves where a GString +// interpolates it. +func jenkinsSuiteStage(script string) string { + return strings.Replace(jenkinsContainerTestStage(script), " stages {", " environment { SUITE = 'smoke' }\n stages {", 1) +} + +// requireJenkinsKeepsBuildNumber checks that, with the build number in +// place of SUITE, which only the Jenkins build knows and ddtest so reads in +// no script, the edited literal still hands the shell want after the first +// command, with the build number still interpolated in a GString. +func requireJenkinsKeepsBuildNumber(t *testing.T, applied, first string, interpolated []string, want string) { + t.Helper() + built := strings.NewReplacer("env.SUITE", "env.BUILD_NUMBER", "$SUITE", "$BUILD_NUMBER").Replace(applied) + written, got := jenkinsWritten(t, built) + require.Equal(t, strings.NewReplacer("env.SUITE", "env.BUILD_NUMBER", "$SUITE", "$BUILD_NUMBER").Replace(first)+want, written) + var expected []string + for _, expr := range interpolated { + expected = append(expected, strings.Replace(expr, "SUITE", "BUILD_NUMBER", 1)) + } + require.Equal(t, expected, got, "Groovy still interpolates the build number") +} + +// jenkinsReplaceCommand replaces, in a Jenkinsfile whose test step's script +// is script(old), the old command as that literal holds it with command as +// ddtest prescribes it, leaving the rest of the literal as it is: what a +// user following the prescription does. +func jenkinsReplaceCommand(t *testing.T, jenkinsfile string, script func(string) string, old, command string) string { + t.Helper() + before, empty := script(old), script("") + // The literal without the command shares its start and end with the + // literal with it: the command is what sits between them. + at := 0 + for at < len(empty) && before[at] == empty[at] { + at++ + } + edited := before[:at] + command + before[len(before)-(len(empty)-at):] + require.Contains(t, jenkinsfile, before) + return strings.Replace(jenkinsfile, before, edited, 1) +} + +// In a container stage, the snippet's stage, a direct Cucumber invocation +// piped into a report converter gets its replacement escaped for the +// string literal its sh step already uses, so replacing only the old +// command inside it hands the shell the exact status subshell while the +// script's other commands, and what a GString interpolates in them, such +// as ${env.BUILD_NUMBER}, stay as they are: switching a GString to single +// quotes would hand the shell ${env.BUILD_NUMBER}, a bad substitution. +// Quotes in the command are kept, and ddtest reads the result as the tests +// and the conversion after them. A host stage gets the plugin, which +// reaches the command without any change. +func TestJenkinsReportPipePrescriptionKeepsTheStepsGroovyLiteral(t *testing.T) { + framework := scopedFrameworks[0] + pipe := `./node_modules/.bin/cucumber-js ./features --format json | ./node_modules/.bin/cucumber-junit --name 'My suite' > junit.xml` + tests := cucumberScopedOptions + " " + cucumberImpactedWorkaround + " npx --no-install nyc -- ./node_modules/.bin/cucumber-js ./features --format json:cucumber-report.json" + conversion := `cat cucumber-report.json | ./node_modules/.bin/cucumber-junit --name 'My suite' > junit.xml` + want := statusSubshell("cucumber-report.json", tests, conversion) + for _, literal := range jenkinsLiterals { + t.Run(literal.name, func(t *testing.T) { + jenkinsfile := jenkinsSuiteStage(literal.script(pipe)) + written, interpolated := jenkinsWritten(t, jenkinsfile) + require.Equal(t, literal.first+pipe, written) + require.Equal(t, literal.interpolated, interpolated) + root := scopedRepository(t, scopedProvider{name: jenkinsProvider, file: "Jenkinsfile"}, jenkinsfile, framework.pkg, nil) + _, err := PlanJavaScriptSetup(root, "cucumber", "5.133.0", "datadoghq.com", ">=18") + var gap *EditThenApplyGap + require.ErrorAs(t, err, &gap) + require.NotContains(t, gap.Reason, "replace that string with") + require.Contains(t, gap.Reason, "replace only the old command inside that string") + _, after, ok := strings.Cut(gap.Reason, "or the conversion's when only the conversion fails: ") + require.True(t, ok, gap.Reason) + command, _, ok := strings.Cut(after, ". Then rerun onboard --apply") + require.True(t, ok, gap.Reason) + + applied := jenkinsReplaceCommand(t, jenkinsfile, literal.script, pipe, command) + require.Equal(t, jenkinsSuiteStage(literal.script(want)), applied) + written, interpolated = jenkinsWritten(t, applied) + require.Equal(t, literal.first+want, written, "Groovy passes the shell the exact command") + require.Equal(t, literal.interpolated, interpolated, "the script's other expressions stay interpolated") + requireJenkinsKeepsBuildNumber(t, applied, literal.first, literal.interpolated, want) + writeFile(t, root, "Jenkinsfile", applied) + requireJenkinsReadsTheStatusSubshell(t, root, tests, conversion) + }) + } + + host := strings.Replace(jenkinsContainerTestStage(groovyQuote(pipe, "'")), "agent { docker { image 'node:22' } }", "agent any", 1) + root := scopedRepository(t, scopedProvider{name: jenkinsProvider, file: "Jenkinsfile"}, host, framework.pkg, nil) + _, err := PlanJavaScriptSetup(root, "cucumber", "5.133.0", "datadoghq.com", ">=18") + require.NoError(t, err, "the plugin needs no change to the command") +} + +// The tests inside an existing status subshell in a container stage get +// their scoped command escaped for the string literal of its sh step, +// which ddtest reads back as the same tests and conversion, with the +// script's other expressions still interpolated. +func TestJenkinsStatusSubshellEditKeepsTheStepsGroovyLiteral(t *testing.T) { + framework := scopedFrameworks[0] + conversion := "cat cucumber-report.json | ./node_modules/.bin/cucumber-junit > junit.xml" + original := statusSubshell("cucumber-report.json", "./node_modules/.bin/cucumber-js ./features --format json:cucumber-report.json", conversion) + tests := cucumberScopedOptions + " " + cucumberImpactedWorkaround + " npx --no-install nyc -- ./node_modules/.bin/cucumber-js ./features --format json:cucumber-report.json" + want := statusSubshell("cucumber-report.json", tests, conversion) + for _, literal := range jenkinsLiterals { + t.Run(literal.name, func(t *testing.T) { + jenkinsfile := jenkinsSuiteStage(literal.script(original)) + written, _ := jenkinsWritten(t, jenkinsfile) + require.Equal(t, literal.first+original, written) + root := scopedRepository(t, scopedProvider{name: jenkinsProvider, file: "Jenkinsfile"}, jenkinsfile, framework.pkg, nil) + _, err := PlanJavaScriptSetup(root, "cucumber", "5.133.0", "datadoghq.com", ">=18") + var gap *EditThenApplyGap + require.ErrorAs(t, err, &gap) + require.NotContains(t, gap.Reason, "replace that string with") + _, after, ok := strings.Cut(gap.Reason, "Change this CI command to exactly: ") + require.True(t, ok, gap.Reason) + command, _, ok := strings.Cut(after, ". This is the command as the sh step's ") + require.True(t, ok, gap.Reason) + + applied := jenkinsReplaceCommand(t, jenkinsfile, literal.script, original, command) + require.Equal(t, jenkinsSuiteStage(literal.script(want)), applied) + written, interpolated := jenkinsWritten(t, applied) + require.Equal(t, literal.first+want, written) + require.Equal(t, literal.interpolated, interpolated, "the script's other expressions stay interpolated") + requireJenkinsKeepsBuildNumber(t, applied, literal.first, literal.interpolated, want) + writeFile(t, root, "Jenkinsfile", applied) + requireJenkinsReadsTheStatusSubshell(t, root, tests, conversion) + }) + } +} + +// requireJenkinsReadsTheStatusSubshell checks that the Jenkinsfile's test +// step is read as the status subshell of tests and conversion, and that +// onboard --apply asks for no further change to its command. +func requireJenkinsReadsTheStatusSubshell(t *testing.T, root, tests, conversion string) { + t.Helper() + scope, err := DiscoverValidationScope(root, "cucumber") + require.NoError(t, err) + require.Empty(t, scope.Unresolved) + require.Len(t, scope.Commands, 1) + _, configuration, _ := strings.Cut(tests, "npx --no-install ") + require.Equal(t, configuration, scope.Commands[0].Command, "the tests are the configuration") + require.Contains(t, scope.Commands[0].StepScopeNotes, "This CI step runs "+conversion+" 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.") + _, err = PlanJavaScriptSetup(root, "cucumber", "5.133.0", "datadoghq.com", ">=18") + if err != nil { + require.NotContains(t, err.Error(), "CI command", "the command needs no further change") + } +} + +// A command escaped for each Groovy string literal is the exact command +// Groovy hands the shell, with backslashes, $, quotes, runs of three quotes +// and line breaks, while a ${…} the literal interpolates before it stays +// interpolated; a script that is not a literal gets the command as it is, +// and other providers but Buildkite get it unchanged. +func TestJenkinsCommandRoundTripsThroughEachGroovyLiteral(t *testing.T) { + commands := []string{`echo "$a" 'b'`, `printf '%s\n' x`, `echo '''`, `echo ''''`, `echo """`, `echo """"`, `echo \\`, `exit "$status"`, `echo "a"`, `echo 'a'`, "echo a\necho b", `echo ${NODE_OPTIONS} $?`} + for _, quote := range []string{`"`, `"""`, "'", "'''"} { + for _, command := range commands { + written, note := jenkinsCommand(quote, command) + require.Contains(t, note, "replace only the old command inside that string") + prefix := "echo ${env.BUILD_NUMBER}; " + var interpolated []string + if strings.HasPrefix(quote, `"`) { + interpolated = []string{"env.BUILD_NUMBER"} + } + text, got := jenkinsWritten(t, "sh "+quote+prefix+written+quote+"\n") + require.Equal(t, prefix+command, text, "%s %s", quote, written) + require.Equal(t, interpolated, got, "%s %s", quote, written) + } + } + written, note := jenkinsCommand("", `echo "$a"`) + require.Equal(t, `echo "$a"`, written) + require.Contains(t, note, "not a Groovy string literal") + written, note = providerCommand(gitLabCI{}, runtimeStep{}, `echo "$a"`) + require.Equal(t, `echo "$a"`, written) + require.Empty(t, note) + written, _ = providerCommand(jenkinsContainer{}, runtimeStep{origin: jenkinsShellStep{quote: `"`}}, `echo "$a"`) + require.Equal(t, `echo \"\$a\"`, written) +} + +// The intermediate report is a file the pipeline names nowhere, so the +// conversion never writes to the file it reads; a quoted formatter option +// is replaced by its decoded text, quoted only when it needs to be. +func TestCucumberReportPipeChoosesAFreshReportAndDecodesTheOption(t *testing.T) { + for command, want := range map[string][2]string{ + "cucumber-js --format junit | tee cucumber-report.xml": {"cucumber-js --format junit:cucumber-report-1.xml", "cat cucumber-report-1.xml | tee cucumber-report.xml"}, + "cucumber-js --format json | cucumber-junit > cucumber-report.json": {"cucumber-js --format json:cucumber-report-1.json", "cat cucumber-report-1.json | cucumber-junit > cucumber-report.json"}, + "cucumber-js --format json | cucumber-junit >> ./cucumber-report-1.json > cucumber-report.json": {"cucumber-js --format json:cucumber-report-2.json", "cat cucumber-report-2.json | cucumber-junit >> ./cucumber-report-1.json > cucumber-report.json"}, + "cucumber-js -f message | cucumber-junit -o cucumber-report.ndjson": {"cucumber-js -f message:cucumber-report-1.ndjson", "cat cucumber-report-1.ndjson | cucumber-junit -o cucumber-report.ndjson"}, + "cucumber-js -f message | cucumber-junit --output=cucumber-report.ndjson": {"cucumber-js -f message:cucumber-report-1.ndjson", "cat cucumber-report-1.ndjson | cucumber-junit --output=cucumber-report.ndjson"}, + "cucumber-js '--format=json' | cucumber-junit": {"cucumber-js --format=json:cucumber-report.json", "cat cucumber-report.json | cucumber-junit"}, + `cucumber-js "--format=json" | cucumber-junit`: {"cucumber-js --format=json:cucumber-report.json", "cat cucumber-report.json | cucumber-junit"}, + "cucumber-js --format 'json' | cucumber-junit": {"cucumber-js --format json:cucumber-report.json", "cat cucumber-report.json | cucumber-junit"}, + } { + t.Run(command, func(t *testing.T) { + tests, conversion, report, ok := cucumberReportPipe(command, noFile) + require.True(t, ok) + require.Equal(t, want, [2]string{tests, conversion}) + _, parsed := parseCIShell(statusSubshell(report, tests, conversion)) + require.True(t, parsed) + }) + } + // The report's name is the same on every provider; Buildkite's is the + // pipeline that holds the command escaped (see + // TestProviderSplitsACucumberReportPipe for the others). + for _, provider := range scopedProvidersNamed(buildkiteProvider) { + t.Run(provider.name, func(t *testing.T) { + framework := scopedFrameworks[0] + pipe := "./node_modules/.bin/cucumber-js ./features '--format=json' | ./node_modules/.bin/cucumber-junit > cucumber-report.json" + root := scopedRepository(t, provider, provider.config("npm ci", pipe), framework.pkg, nil) + _, err := PlanJavaScriptSetup(root, "cucumber", "5.133.0", "datadoghq.com", ">=18") + var gap *EditThenApplyGap + require.ErrorAs(t, err, &gap) + _, after, ok := strings.Cut(gap.Reason, "or the conversion's when only the conversion fails: ") + require.True(t, ok, gap.Reason) + command, _, ok := strings.Cut(after, ". Then rerun onboard --apply") + require.True(t, ok, gap.Reason) + tests := cucumberScopedOptions + " " + cucumberImpactedWorkaround + " npx --no-install nyc -- ./node_modules/.bin/cucumber-js ./features --format=json:cucumber-report-1.json" + want := statusSubshell("cucumber-report-1.json", tests, "cat cucumber-report-1.json | ./node_modules/.bin/cucumber-junit > cucumber-report.json") + if provider.name == buildkiteProvider { + want = buildkiteEscape(want) // As the pipeline holds it. + } + require.Equal(t, want, command) + + writeFile(t, root, provider.file, provider.config("npm ci", command)) + _, err = PlanJavaScriptSetup(root, "cucumber", "5.133.0", "datadoghq.com", ">=18") + require.NoError(t, err) + }) + } +} + +// noFile is a directory with no file of any name. +func noFile(string) bool { return false } + +// The intermediate report also names no file of the step's directory, such +// as a tracked fixture the tests would overwrite, and onboard --apply accepts +// the prescription that skips it. +func TestCucumberReportPipeSkipsTheStepDirectorysFiles(t *testing.T) { + // Every provider reads the step directory the same way, here the + // repository root (see TestReportNameTakenReadsTheStepDirectory for + // another). + for _, provider := range scopedProvidersNamed(circleCIProvider) { + t.Run(provider.name, func(t *testing.T) { + framework := scopedFrameworks[0] + pipe := "./node_modules/.bin/cucumber-js ./features --format json | ./node_modules/.bin/cucumber-junit > junit.xml" + root := scopedRepository(t, provider, provider.config("npm ci", pipe), framework.pkg, map[string]string{"cucumber-report.json": "{}\n", "cucumber-report-1.json": "{}\n"}) + _, err := PlanJavaScriptSetup(root, "cucumber", "5.133.0", "datadoghq.com", ">=18") + var gap *EditThenApplyGap + require.ErrorAs(t, err, &gap) + _, after, ok := strings.Cut(gap.Reason, "or the conversion's when only the conversion fails: ") + require.True(t, ok, gap.Reason) + command, _, ok := strings.Cut(after, ". Then rerun onboard --apply") + require.True(t, ok, gap.Reason) + tests := cucumberScopedOptions + " " + cucumberImpactedWorkaround + " npx --no-install nyc -- ./node_modules/.bin/cucumber-js ./features --format json:cucumber-report-2.json" + want := statusSubshell("cucumber-report-2.json", tests, "cat cucumber-report-2.json | ./node_modules/.bin/cucumber-junit > junit.xml") + if provider.name == buildkiteProvider { + want = buildkiteEscape(want) // As the pipeline holds it. + } + require.Equal(t, want, command) + + writeFile(t, root, provider.file, provider.config("npm ci", command)) + plan, err := PlanJavaScriptSetup(root, "cucumber", "5.133.0", "datadoghq.com", ">=18") + require.NoError(t, err) + require.Contains(t, plannedFile(t, plan, provider.file), command) + }) + } +} + +// A report name is taken by a file of the step's directory, or a path git +// tracks there that the checkout lacks, and by no file elsewhere. +func TestReportNameTakenReadsTheStepDirectory(t *testing.T) { + root := t.TempDir() + output, err := exec.Command("git", "init", "-q", root).CombinedOutput() + require.NoError(t, err, string(output)) + writeFile(t, root, "cucumber-report.json", "{}\n") + writeFile(t, root, "packages/a/cucumber-report.xml", "\n") + writeFile(t, root, "packages/a/cucumber-report.html", "\n") + output, err = exec.Command("git", "-C", root, "add", "packages/a/cucumber-report.html").CombinedOutput() + require.NoError(t, err, string(output)) + require.NoError(t, os.Remove(filepath.Join(root, "packages/a/cucumber-report.html"))) + + taken := reportNameTaken(root, ".") + require.True(t, taken("cucumber-report.json")) + require.False(t, taken("cucumber-report.xml")) + taken = reportNameTaken(root, "packages/a") + require.False(t, taken("cucumber-report.json"), "the root's file is not the step's") + require.True(t, taken("cucumber-report.xml")) + require.True(t, taken("cucumber-report.html"), "tracked, though missing from the checkout") +} + +// The status subshell records the statuses in variables neither its tests +// nor its conversion reference: status and converted, else datadog_status +// and datadog_converted, then those with a numeric suffix. +func TestStatusVariablesAvoidThePipesVariables(t *testing.T) { + for commands, want := range map[[2]string][2]string{ + {"cucumber-js --format json:r.json", "cat r.json | cucumber-junit"}: {"status", "converted"}, + {"cucumber-js status --format json:r.json", "cat r.json | cucumber-junit > converted.xml"}: {"status", "converted"}, + {`cucumber-js "$status" --format json:r.json`, "cat r.json | cucumber-junit"}: {"datadog_status", "datadog_converted"}, + {"cucumber-js --format json:r.json", `cat r.json | cucumber-junit --name "${converted}"`}: {"datadog_status", "datadog_converted"}, + {"cucumber-js $((status + 1)) --format json:r.json", "cat r.json | cucumber-junit"}: {"datadog_status", "datadog_converted"}, + {"cucumber-js --format json:r.json", "cat r.json | status=1 cucumber-junit"}: {"datadog_status", "datadog_converted"}, + {`cucumber-js "$status" "$datadog_converted" --format json:r.json`, "cat r.json | cucumber-junit"}: {"datadog_status_2", "datadog_converted_2"}, + {`cucumber-js "$status" "$datadog_status_2"`, `cat r.json | cucumber-junit "$datadog_status"`}: {"datadog_status_3", "datadog_converted_3"}, + } { + status, converted := statusVariables(commands[0], commands[1]) + require.Equal(t, want, [2]string{status, converted}, commands) + command := statusSubshell("r.json", commands[0], commands[1]) + require.True(t, strings.HasPrefix(command, "("+want[0]+"=0; rm -f r.json; "), command) + require.True(t, strings.HasSuffix(command, ` || { `+want[1]+`=$?; [ "$`+want[0]+`" -ne 0 ] || `+want[0]+`=$`+want[1]+`; }; exit "$`+want[0]+`")`), command) + } +} + +// Tests that read the job's status variable keep reading it inside the +// status subshell ddtest writes for them, which records into other +// variables, on every provider whose scripts stop at the first failing +// command (Buildkite's with $$): onboarding reads its tests with the job's +// value. In a subshell that records into status itself, the tests would +// read the subshell's 0 instead, and the command stays unresolved. +func TestEveryProviderReadsTheJobsStatusVariableInsideTheSubshell(t *testing.T) { + tests := `./node_modules/.bin/cucumber-js ./features --tags "$status" --format json:cucumber-report.json` + conversion := "cat cucumber-report.json | ./node_modules/.bin/cucumber-junit > junit.xml" + shadowed := `(status=0; rm -f cucumber-report.json; ` + tests + ` || status=$?; ` + conversion + ` || { converted=$?; [ "$status" -ne 0 ] || status=$converted; }; exit "$status")` + command := statusSubshell("cucumber-report.json", tests, conversion) + require.Equal(t, `(datadog_status=0; rm -f cucumber-report.json; `+tests+` || datadog_status=$?; `+conversion+` || { datadog_converted=$?; [ "$datadog_status" -ne 0 ] || datadog_status=$datadog_converted; }; exit "$datadog_status")`, command) + for name, write := range map[string]func(root, command string){ + gitLabCIProvider: func(root, command string) { + writeFile(t, root, gitLabCIFile, "test:\n image: node:22\n variables:\n status: smoke\n script:\n - npm ci\n - "+command+"\n") + }, + circleCIProvider: func(root, command string) { + writeFile(t, root, circleCIFile, "version: 2.1\njobs:\n test:\n docker:\n - image: cimg/node:22.11\n environment:\n status: smoke\n steps:\n - checkout\n - run: npm ci\n - run: "+command+"\n"+circleWorkflow(" - test\n")) + }, + buildkiteProvider: func(root, command string) { + writeFile(t, root, buildkiteFile, "steps:\n - label: test\n env:\n status: smoke\n command:\n - npm ci\n - "+buildkiteEscape(command)+"\n") + }, + jenkinsProvider: func(root, command string) { + writeFile(t, root, "Jenkinsfile", "pipeline {\n agent any\n environment {\n status = 'smoke'\n }\n stages {\n stage('Test') {\n steps {\n sh 'npm ci'\n sh '''\n "+command+"\n '''\n }\n }\n }\n}\n") + }, + } { + t.Run(name, func(t *testing.T) { + root := t.TempDir() + writeFile(t, root, "package.json", `{"devDependencies":{"@cucumber/cucumber":"11.0.1","cucumber-junit":"1.7.1"}}`) + t.Chdir(root) + useCIProvider(t, name) + write(root, command) + scope, err := DiscoverValidationScope(root, "cucumber") + require.NoError(t, err) + require.Empty(t, scope.Unresolved) + require.Len(t, scope.Commands, 1) + require.Equal(t, "cucumber-js ./features --tags smoke --format json:cucumber-report.json", scope.Commands[0].Command, "the job's status") + require.Contains(t, scope.Commands[0].StepScopeNotes, "This CI step runs "+conversion+" 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.") + + write(root, shadowed) + scope, err = DiscoverValidationScope(root, "cucumber") + require.NoError(t, err) + require.Empty(t, scope.Commands, "the tests would read the subshell's status") + require.NotEmpty(t, scope.Unresolved) + }) + } +} + +// In a real shell, the tests and the conversion get the values CI gave +// status and converted, whatever the subshell records, and the command +// still exits with the tests' status, or the conversion's when only it +// fails: in sh with errexit on and off, and in bash and dash with errexit. +func TestStatusSubshellKeepsTheShellsStatusVariable(t *testing.T) { + conversion := `cat report.json | sh -c 'cat > junit.xml; echo "conversion $1" >> junit.xml; exit 5' _ "$converted"` + for _, shell := range realShells(t, "sh", "bash", "dash") { + var cases []shellCase + var labels []string + var wants []int + options := []string{"set -e"} + if shell == "sh" { + options = append(options, "set +e") + } + for _, options := range options { + for tests, want := range map[string]int{ + `sh -c 'echo "tests $1" > report.json' _ "$status"`: 5, + `sh -c 'echo "tests $1" > report.json; exit 3' _ "$status"`: 3, + } { + command := statusSubshell("report.json", tests, conversion) + require.True(t, strings.HasPrefix(command, "(datadog_status=0; "), command) + cases = append(cases, shellCase{script: options + "\nstatus=ci-status\nconverted=ci-converted\n" + command + "\n"}) + labels = append(labels, shell+" "+options+" "+tests) + wants = append(wants, want) + } + } + for i, result := range runShellCases(t, shell, cases) { + assert.Equal(t, wants[i], result.status, labels[i]) + junit, err := os.ReadFile(filepath.Join(result.dir, "junit.xml")) + if assert.NoError(t, err, labels[i]) { + assert.Equal(t, "tests ci-status\nconversion ci-converted\n", string(junit), labels[i]) + } + } + } +} diff --git a/internal/onboard/runtime.go b/internal/onboard/runtime.go index d3126bbe..af9d2455 100644 --- a/internal/onboard/runtime.go +++ b/internal/onboard/runtime.go @@ -168,6 +168,14 @@ type runtimeStep struct { // impactedWorkaround reports Cucumber's impacted-tests workaround right // after that prefix (see readScopedPrefix). impactedWorkaround bool + // afterTests are the commands a status-preserving subshell runs after + // Run's tests whatever their result, before it exits with their status, + // or the status of one that fails when they pass (see + // readStatusSubshell). + afterTests []string + // statusWrapper is the text of that subshell before and after its + // tests, which an edit to the tests keeps (see directScopedCommand). + statusWrapper [2]string // inline names the variables assigned inline before Run's command, which // the provider's reader moved into Env (see foldInlineEnvironment). inline []string diff --git a/internal/onboard/scripts.go b/internal/onboard/scripts.go index c8b7e648..6eaf36de 100644 --- a/internal/onboard/scripts.go +++ b/internal/onboard/scripts.go @@ -79,6 +79,37 @@ func unresolvedCommand(reason string) commandResolution { } func resolveTestStep(root string, workflow ciWorkflow, job runtimeJob, step runtimeStep, language, framework string) commandResolution { + if len(step.afterTests) > 0 { + // The commands a status-preserving subshell runs after its tests are + // outside them only when they can't start the tests themselves. + plain := step + plain.afterTests = nil + for _, command := range step.afterTests { + after := plain + after.Run, after.scopedVariable, after.impactedWorkaround = command, "", false + after.Env = maps.Clone(step.Env) + for _, name := range step.inline { + delete(after.Env, name) // The tests' own inline variables. + } + if stepMayStartTests(root, workflow, job, after, framework) { + return unresolvedCommand("CI runs " + command + " after the tests in the same command, and it may start " + framework + " itself; ddtest reads only commands that can't start tests there") + } + } + // They are read as the same commands after the tests in a plain + // sequence are: file changes stay setup for later tests, and those + // ddtest can't resolve are outside the tests unless a later test + // follows them. + sequence := plain + sequence.Run = strings.Join(append([]string{plain.Run}, step.afterTests...), "\n") + result := resolveTestStep(root, workflow, job, sequence, language, framework) + note := "This CI step runs " + strings.Join(step.afterTests, "; ") + " after its " + framework + " 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." + for i := range result.Tests { + if !slices.Contains(result.Tests[i].StepScopeNotes, note) { + result.Tests[i].StepScopeNotes = append(result.Tests[i].StepScopeNotes, note) + } + } + return result + } if step.scopedVariable != "" { plain := step plain.scopedVariable = "" diff --git a/internal/testdrive/apply_partial_test.go b/internal/testdrive/apply_partial_test.go index 63eefce6..da3d7415 100644 --- a/internal/testdrive/apply_partial_test.go +++ b/internal/testdrive/apply_partial_test.go @@ -121,3 +121,73 @@ func TestWorkflowProceedsWithResolvedConfigurationsAndReportsUnresolvedCI(t *tes require.Contains(t, scopeCheck.Reason, "unknown-wrapper.js") } } + +// A framework whose CI setup onboard --apply doesn't write (Cypress) hands +// off to the exact manual setup: next_command prints it, the result names +// the release to write, and resume_command validates it. +func TestWorkflowSendsManualSetupFrameworksToTheInstructions(t *testing.T) { + t.Setenv("TMPDIR", t.TempDir()) + root := t.TempDir() + t.Chdir(root) + onboard.UseCIProvider("gitlab") + t.Cleanup(func() { onboard.UseCIProvider("") }) + requireWriteFile(t, filepath.Join(root, "package.json"), `{"scripts":{"e2e":"cypress run"},"devDependencies":{"cypress":"15.1.0"}}`) + requireWriteFile(t, filepath.Join(root, "cypress.config.js"), "module.exports = { e2e: { supportFile: false } }\n") + requireWriteFile(t, filepath.Join(root, ".gitlab-ci.yml"), "e2e:\n image: cypress/base:24.19.0\n script:\n - npm ci\n - npm run e2e\n") + drive := &Testdrive{repositoryRoot: root, framework: &framework.Cypress{}, language: "javascript", nodeVersion: func() string { return "24.19.0" }} + drive.workflowSelect = func(context.Context, onboard.ValidationScope, bool) (string, error) { return "6.23.0", nil } + var output bytes.Buffer + require.Error(t, drive.Workflow(t.Context(), &output, WorkflowOptions{Apply: true, Format: "json"})) + var result onboard.AgentResult + require.NoError(t, json.Unmarshal(output.Bytes(), &result)) + require.Equal(t, "ci_edit_unsupported", result.Code, output.String()) + require.Equal(t, "run_command", result.NextAction) + require.Contains(t, strings.Join(result.NextCommand, " "), "onboard --framework cypress --instructions --ci-provider gitlab") + require.Contains(t, result.Reason, "onboard --apply does not write the Cypress CI setup on GitLab CI") + require.Contains(t, result.Reason, "dd-trace 6.23.0 in place of its tracer release placeholder ("+onboard.CIProviderTracerSetting("js", "6.23.0")+")") + require.Contains(t, result.Needs, "Do not write a loader, hook or plugin integration of your own") + require.NotContains(t, result.Reason+result.Needs+strings.Join(result.RequiredInput, " "), "loader/hook integration") + require.Contains(t, strings.Join(result.ResumeCommand, " "), "testdrive --framework cypress --all --agent --tracer-version 6.23.0") + require.Empty(t, result.Changes) +} + +// The manual setup instructions show the default site: with another site +// selected, the handoff names it in place of the example, and resume_command +// validates that site. +func TestWorkflowManualSetupHandoffNamesTheSelectedSite(t *testing.T) { + for _, provider := range []string{"github", "gitlab"} { + t.Run(provider, func(t *testing.T) { + t.Setenv("TMPDIR", t.TempDir()) + root := t.TempDir() + t.Chdir(root) + onboard.UseCIProvider(provider) + t.Cleanup(func() { onboard.UseCIProvider("") }) + requireWriteFile(t, filepath.Join(root, "package.json"), `{"scripts":{"e2e":"cypress run"},"devDependencies":{"cypress":"15.1.0"}}`) + requireWriteFile(t, filepath.Join(root, "cypress.config.js"), "module.exports = { e2e: { supportFile: false } }\n") + if provider == "github" { + require.NoError(t, os.MkdirAll(filepath.Join(root, ".github/workflows"), 0o755)) + requireWriteFile(t, filepath.Join(root, ".github/workflows/test.yml"), "jobs:\n e2e:\n runs-on: ubuntu-latest\n steps:\n - uses: actions/setup-node@v4\n with:\n node-version: 24\n - run: npm ci\n - run: npm run e2e\n") + } else { + requireWriteFile(t, filepath.Join(root, ".gitlab-ci.yml"), "e2e:\n image: cypress/base:24.19.0\n script:\n - npm ci\n - npm run e2e\n") + } + drive := &Testdrive{repositoryRoot: root, framework: &framework.Cypress{}, language: "javascript", nodeVersion: func() string { return "24.19.0" }} + drive.workflowSelect = func(context.Context, onboard.ValidationScope, bool) (string, error) { return "6.23.0", nil } + var output bytes.Buffer + require.Error(t, drive.Workflow(t.Context(), &output, WorkflowOptions{Apply: true, Format: "json", Site: "datadoghq.eu"})) + var result onboard.AgentResult + require.NoError(t, json.Unmarshal(output.Bytes(), &result)) + require.Equal(t, "ci_edit_unsupported", result.Code, output.String()) + setting := "the Datadog site datadoghq.eu in place of the example site datadoghq.com, and dd-trace 6.23.0 in place of its tracer release placeholder" + require.Contains(t, result.Reason, setting) + require.Contains(t, result.Needs, setting) + require.Contains(t, strings.Join(result.RequiredInput, " "), setting) + require.NotContains(t, result.NextCommand, "--site", "onboard --instructions takes no setup options") + require.Contains(t, strings.Join(result.ResumeCommand, " "), "--site datadoghq.eu") + require.Equal(t, "datadoghq.eu", result.PlanSummary.Site) + }) + } + // The default site needs no substitution. + for _, site := range []string{"", "datadoghq.com"} { + require.Empty(t, onboard.CIProviderSiteSetting(site)) + } +} diff --git a/internal/testdrive/handoff_gaps_test.go b/internal/testdrive/handoff_gaps_test.go index 7662e725..04b216e2 100644 --- a/internal/testdrive/handoff_gaps_test.go +++ b/internal/testdrive/handoff_gaps_test.go @@ -211,8 +211,13 @@ func TestManualCITracerFitsUnresolvedStepsWithAKnownNode(t *testing.T) { // CI Node runs, which those edits pin. func TestScopeUnresolvedNeedsKeepTheManualTracer(t *testing.T) { prior := "Facts can add prerequisites." + manualCITracerPrefix + `DD_SET_TRACER_VERSION_JS="5.133.0" (every detected CI Node runtime supports it) and pass --tracer-version 5.133.0 to validation.` - needs := scopeUnresolvedNeeds(prior) + needs := scopeUnresolvedNeeds(prior, "") require.True(t, strings.HasPrefix(needs, "Run next_command for the unsupported CI integration")) require.True(t, strings.HasSuffix(needs, `For manual CI edits use DD_SET_TRACER_VERSION_JS="5.133.0" (every detected CI Node runtime supports it) and pass --tracer-version 5.133.0 to validation.`)) - require.NotContains(t, scopeUnresolvedNeeds("Facts can add prerequisites."), "manual CI edits") + require.NotContains(t, scopeUnresolvedNeeds("Facts can add prerequisites.", ""), "manual CI edits") + for _, site := range []string{"", "datadoghq.com"} { + require.NotContains(t, scopeUnresolvedNeeds(prior, site), "Datadog site", "the instructions already show the default site") + } + // The instructions show the default site: a selected one is named. + require.True(t, strings.HasSuffix(scopeUnresolvedNeeds(prior, "datadoghq.eu"), " In CI setup written by hand, use the Datadog site datadoghq.eu in place of the example site datadoghq.com that onboard --instructions shows.")) } diff --git a/internal/testdrive/java_features_test.go b/internal/testdrive/java_features_test.go index 67047dbf..018ddd96 100644 --- a/internal/testdrive/java_features_test.go +++ b/internal/testdrive/java_features_test.go @@ -19,8 +19,10 @@ import ( "testing" "github.com/DataDog/ddtest/internal/framework" + "github.com/DataDog/ddtest/internal/settings" "github.com/DataDog/ddtest/internal/testdrive/intake" "github.com/DataDog/ddtest/internal/testdrive/workspace" + "github.com/spf13/viper" "github.com/stretchr/testify/require" ) @@ -428,6 +430,34 @@ func TestJavaValidationReportsTheCIPinnedAgent(t *testing.T) { require.Equal(t, "compatible", result.CIRuntime.Status, "%+v", result.CIRuntime) } +// An explicit --tracer-version latest validates the exact latest release, +// which the setup onboarding prescribed pins in CI, so the pinned row is +// compatible, not a mismatch between latest and that release. +func TestJavaValidationResolvesAnExplicitLatest(t *testing.T) { + root := t.TempDir() + requireWriteFile(t, filepath.Join(root, "pom.xml"), `fixtureorg.junit.jupiterjunit-jupiter`) + t.Chdir(root) + viper.Reset() + settings.Init() + t.Cleanup(func() { viper.Reset(); settings.Init() }) + prepared, err := Prepare(PrepareOptions{TracerVersion: "latest", CheckOnly: true}) + require.NoError(t, err) + require.Equal(t, "1.68.1", prepared.tracerVersion, "the release every Java setup of this run pins") + require.Equal(t, "dd-java-agent@1.68.1", prepared.tracerLabel) + + drive, _, _ := javaSimulation(t) + require.NoError(t, os.MkdirAll(filepath.Join(drive.repositoryRoot, ".github/workflows"), 0755)) + requireWriteFile(t, filepath.Join(drive.repositoryRoot, ".github/workflows/ci.yml"), "jobs:\n test:\n steps:\n - uses: actions/setup-java@v4\n with: {java-version: '17'}\n - uses: datadog/test-visibility-github-action@v3\n with: {languages: java, api_key: '${{ secrets.DD_API_KEY }}', java-tracer-version: '1.68.1'}\n - run: mvn -B test\n") + drive.configurationEnvironment = map[string]string{"JAVA_HOME": writeJDK(t, "17.0.20")} + drive.platform = &fakeTracer{preloadPath: "/agent/dd-java-agent.jar"} + drive.tracerVersion, drive.tracerLabel = prepared.tracerVersion, prepared.tracerLabel + drive.preflight = drive.frameworkPreflight() + var output bytes.Buffer + result, err := drive.run(t.Context(), &output) + require.NoError(t, err, output.String()) + require.Equal(t, "compatible", result.CIRuntime.Status, "%+v", result.CIRuntime) +} + func TestJavaFeaturesPassWhenSurefireReportsNoSkippedProbe(t *testing.T) { drive, maven, session := javaSimulation(t) maven.unreportedSkips = true diff --git a/internal/testdrive/onboard.go b/internal/testdrive/onboard.go index 2a455c97..c653b00b 100644 --- a/internal/testdrive/onboard.go +++ b/internal/testdrive/onboard.go @@ -151,7 +151,7 @@ func (t *Testdrive) Workflow(ctx context.Context, output io.Writer, options Work result.NextAction = "run_command" result.NextCommand = onboard.DDTestCommand("onboard", "--framework", name, "--instructions") result.ResumeCommand, result.RequiredInput = nil, nil - result.Needs = scopeUnresolvedNeeds(result.Needs) + result.Needs = scopeUnresolvedNeeds(result.Needs, options.Site) case "setup_dependencies_missing", "project_dependencies_missing": result.NextAction = "run_command" case "ci_edit_required", "report_exclusions_required": @@ -164,7 +164,11 @@ func (t *Testdrive) Workflow(ctx context.Context, output io.Writer, options Work result.NextAction = "supply_input" } case "unsupported": - if code == "ci_edit_unsupported" { + if code == "ci_edit_unsupported" && len(result.NextCommand) > 0 { + // The setup to apply comes from next_command (the manual + // instructions); resume_command validates it. + result.NextAction = "run_command" + } else if code == "ci_edit_unsupported" { result.NextAction = "apply_source_edit" } else { result.NextAction = "report_result" @@ -501,14 +505,23 @@ func (t *Testdrive) Workflow(ctx context.Context, output io.Writer, options Work result.Checks = append(result.Checks, onboard.WorkflowCheck{Name: "preflight_preparation", Status: "passed", Reason: fmt.Sprintf("Verified preflight reused: %t. Test execution is always fresh.", reused)}) t.setPlanTracer(plan, selected) if !onboard.SupportsAutomaticSetup(name) { + // The manual setup instructions hold the exact CI setup, as for + // unresolved scope; only the tracer release they leave to the + // reader is named here. + setting := "dd-trace " + selected + " in place of its tracer release placeholder (" + onboard.CIProviderTracerSetting("js", selected) + ")" + if site := onboard.CIProviderSiteSetting(plan.Site); site != "" { + // The instructions show the default site; resume_command + // validates the selected one. + setting = site + ", and " + setting + } result.Location = "CI initialization" - result.RequiredInput = []string{"framework-specific CI loader/hook integration using the selected tracer release " + selected} + result.RequiredInput = []string{"the " + onboard.CIProviderDisplayName() + " setup next_command prints, exactly as shown, with " + setting} result.ResumeCommand = t.workflowValidationResume(recoveryCommand[0], name, selected, options) checkpoint.Apply = false checkpoint.All = true result.ResumeCommand = append(result.ResumeCommand, "--resume", checkpoint.RunID) - result.NextCommand = nil - return fail("unsupported", "ci_edit_unsupported", "The selected tracer and runtime are resolved; this framework still needs its custom CI hooks.", "Apply only plan_summary.initialization using plan_summary.tracer and plan_summary.site at the named CI hook; run resume_command. Inspect details_command only if the missing source integration needs more context.") + result.NextCommand = onboard.DDTestCommand("onboard", "--framework", name, "--instructions") + return fail("unsupported", "ci_edit_unsupported", "onboard --apply does not write the "+displayName(name)+" CI setup on "+onboard.CIProviderDisplayName()+". next_command prints it: apply that setup exactly as shown, with "+setting+": what ddtest selected and resume_command validates.", "Run next_command, apply only the CI setup it prints, exactly as shown and with "+setting+", then run resume_command. Do not write a loader, hook or plugin integration of your own, and do not run testdrive or the tests first.") } progress("apply") var sourcePlan *onboard.SetupPlan @@ -575,7 +588,7 @@ func (t *Testdrive) Workflow(ctx context.Context, output io.Writer, options Work result.ResumeCommand = append(result.ResumeCommand, "--allow-dirty") } result.NextCommand = nil - return fail("needs_input", "ci_edit_required", err.Error(), "Make only the edit reason names, then run resume_command: onboard --apply adds the Datadog CI setup and validates. Do not run testdrive first.") + return fail("needs_input", "ci_edit_required", err.Error(), "Make only the edit reason names, then run resume_command: onboard --apply adds the Datadog CI setup and validates. Do not run testdrive first."+manualSiteSentence(options.Site)) } var gap *onboard.SetupGap if errors.As(err, &gap) { @@ -586,7 +599,7 @@ func (t *Testdrive) Workflow(ctx context.Context, output io.Writer, options Work checkpoint.All = true result.ResumeCommand = append(result.ResumeCommand, "--resume", checkpoint.RunID) result.NextCommand = nil - return fail("unsupported", "ci_edit_unsupported", err.Error(), "Integrate only this CI hook, then run resume_command. Do not repeatedly retry the automatic editor or drop configurations.") + return fail("unsupported", "ci_edit_unsupported", err.Error(), "Integrate only this CI hook, then run resume_command. Do not repeatedly retry the automatic editor or drop configurations."+manualSiteSentence(options.Site)) } return fail("blocked_environment", "ci_edit_unavailable", err.Error(), "Restore filesystem access. Completed source edits are listed in changes; do not overwrite customer edits.") } @@ -1349,12 +1362,22 @@ const manualCITracerPrefix = " For manual CI edits use " // scopeUnresolvedNeeds sends unresolved CI to the manual instructions, // keeping the tracer release every CI Node runs that prior named for them. -func scopeUnresolvedNeeds(prior string) string { +func scopeUnresolvedNeeds(prior, site string) string { needs := "Run next_command for the unsupported CI integration, then validate the discovered commands with testdrive. Keep unresolved CI explicitly unvalidated and report it. " + onboard.ScopeBlockerInstruction + " Setup facts cannot resolve CI parsing or shrink scope; do not guess a facts schema, inspect the binary, or retry unchanged --apply." if _, tracer, found := strings.Cut(prior, manualCITracerPrefix); found { needs += manualCITracerPrefix + tracer } - return needs + return needs + manualSiteSentence(site) +} + +// manualSiteSentence names the selected site for CI setup written by hand +// from onboard --instructions, which shows the default site; empty for that +// default. +func manualSiteSentence(site string) string { + if setting := onboard.CIProviderSiteSetting(site); setting != "" { + return " In CI setup written by hand, use " + setting + " that onboard --instructions shows." + } + return "" } func (t *Testdrive) setPlanTracer(plan *onboard.ExecutionPlan, tracer string) { diff --git a/internal/testdrive/resolved_plan_test.go b/internal/testdrive/resolved_plan_test.go index be574388..5b0c5dfd 100644 --- a/internal/testdrive/resolved_plan_test.go +++ b/internal/testdrive/resolved_plan_test.go @@ -279,7 +279,8 @@ func TestCustomHookHandoffContainsResolvedTracerAndResumableScope(t *testing.T) require.ErrorContains(t, drive.Workflow(t.Context(), &output, WorkflowOptions{Apply: true, Format: "json"}), "ci_edit_unsupported") var result onboard.AgentResult require.NoError(t, json.Unmarshal(output.Bytes(), &result)) - require.Equal(t, "apply_source_edit", result.NextAction) + require.Equal(t, "run_command", result.NextAction, "next_command prints the setup to apply") + require.Contains(t, result.NextCommand, "--instructions") require.Equal(t, 1, selections) require.Equal(t, "5.132.0", result.PlanSummary.Tracer) require.Contains(t, result.ResumeCommand, "--tracer-version") diff --git a/internal/testdrive/temp_main_test.go b/internal/testdrive/temp_main_test.go index 8fd614da..ea9bb08d 100644 --- a/internal/testdrive/temp_main_test.go +++ b/internal/testdrive/temp_main_test.go @@ -1,12 +1,17 @@ package testdrive import ( + "context" "fmt" "os" "testing" + + "github.com/DataDog/ddtest/internal/onboard" ) func TestMain(m *testing.M) { + // Java setups name a fixed latest dd-java-agent release, read offline. + onboard.SetJavaReleaseResolver(func(context.Context) (string, error) { return "1.68.1", nil }) if os.Getenv("DDTEST_ISOLATED_TEST_TMP") == "1" { os.Exit(m.Run()) } diff --git a/internal/testdrive/testdrive.go b/internal/testdrive/testdrive.go index e47ba32b..dc433102 100644 --- a/internal/testdrive/testdrive.go +++ b/internal/testdrive/testdrive.go @@ -188,6 +188,13 @@ func Prepare(configuration PrepareOptions) (*Testdrive, error) { return nil, err } language := detectedPlatform.Name() + if !configuration.DefaultTracerVersion { + // An explicit release is the one setup messages name, as onboarding's, + // and, for Java, latest the exact release they pin. + if version, err = onboard.SelectTracerRelease(language, version); err != nil { + return nil, err + } + } selectedLanguage := languageNamed(language) if selectedLanguage == nil { selectedLanguage = unsupportedLanguage{}