Skip to content

roachtest: add github issue posting dry run mode#154164

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
DarrylWong:github-dry-run
Sep 29, 2025
Merged

roachtest: add github issue posting dry run mode#154164
craig[bot] merged 1 commit intocockroachdb:masterfrom
DarrylWong:github-dry-run

Conversation

@DarrylWong
Copy link
Copy Markdown
Contributor

We already have the functionality in datadriven tests to render github issue post requests as clickable links. This change extends that functionality to roachtests in the form of a --dry-run-issue-posting flag. This allows for viewing what the output of a failed roachtest would look like even if github posting is disabled.

Fixes: none
Release note: none
Epic: none

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@DarrylWong DarrylWong marked this pull request as ready for review September 25, 2025 16:14
@DarrylWong DarrylWong requested a review from a team as a code owner September 25, 2025 16:14
@DarrylWong DarrylWong requested review from nameisbhaskar and shailendra-patel and removed request for a team September 25, 2025 16:14
Copy link
Copy Markdown
Contributor

@williamchoe3 williamchoe3 left a comment

Choose a reason for hiding this comment

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

Thanks for this!

if skipReason != "" {
l.Printf("skipping GitHub issue posting (%s)", skipReason)
return nil, nil
if !g.dryRun {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: this would be more verbose, but i feel like having a maybePostDryRun function would make this a little bit easier to follow vs the branching bool logic, but the branches aren't that crazy so ok with how it is now

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done.

@herkolategan
Copy link
Copy Markdown
Collaborator

LGTM, barring the crlfmt fix:

=== RUN   TestLint/TestGofmtSimplify=== PAUSE TestLint/TestGofmtSimplify=== CONT  TestLint/TestGofmtSimplify    gen-lint_test.go:1749:         /github-actions-runner/_work/cockroach/cockroach/pkg/cmd/roachtest/roachtestflags/flags.go        diff /github-actions-runner/_work/cockroach/cockroach/pkg/cmd/roachtest/roachtestflags/flags.go.orig /github-actions-runner/_work/cockroach/cockroach/pkg/cmd/roachtest/roachtestflags/flags.go        --- /github-actions-runner/_work/cockroach/cockroach/pkg/cmd/roachtest/roachtestflags/flags.go.orig        +++ /github-actions-runner/_work/cockroach/cockroach/pkg/cmd/roachtest/roachtestflags/flags.go        @@ -16,7 +16,6 @@         	"github.com/spf13/pflag"         )                 -         // This block defines all roachtest flags (for the list and run/bench commands).         var (         	Cloud spec.Cloud = spec.GCE--- FAIL: TestLint/TestGofmtSimplify (8.22s)

@DarrylWong DarrylWong force-pushed the github-dry-run branch 3 times, most recently from bb6d106 to 6d19f79 Compare September 26, 2025 16:16
We already have the functionality in datadriven tests
to render github issue post requests as clickable links.
This change extends that functionality to roachtests in
the form of a --dry-run-issue-posting flag. This allows
for viewing what the output of a failed roachtest would
look like even if github posting is disabled.
@DarrylWong
Copy link
Copy Markdown
Contributor Author

TFTR!

bors r=williamchoe3, herkolategan

@DarrylWong
Copy link
Copy Markdown
Contributor Author

looks like bors skipped me? 😕

bors cancel

@DarrylWong
Copy link
Copy Markdown
Contributor Author

bors r=williamchoe3,herkolategan

@craig craig bot merged commit a9fe917 into cockroachdb:master Sep 29, 2025
22 of 23 checks passed
@craig
Copy link
Copy Markdown
Contributor

craig bot commented Sep 29, 2025

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants