Skip to content

[oadp-1.5] OADP-8508: Fix schedule create examples showing velero instead of oc oadp #246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-bot merged 1 commit into from
Jul 31, 2026
+41 −0
Merged

[oadp-1.5] OADP-8508: Fix schedule create examples showing velero instead of oc oadp #246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions cmd/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,14 @@ var veleroCommandPattern = regexp.MustCompile(`(?m)(?:^|[\s\x60])velero\s+(?:` +
`(?:version|install|uninstall|plugin|snapshot-location|backup-location|restic|repo|client|completion|bug|debug|datamover)` +
`)`)

// veleroCreateVerbFirstPattern matches upstream Velero's "velero create <resource>" example
// text (verb-first), which some commands hardcode instead of the actual resource-first
// invocation. For example, velero's schedule create command is invoked as
// "velero schedule create NAME", but its upstream Example text says "velero create schedule
// NAME". veleroCommandPattern above doesn't catch this ordering, so it's normalized to
// resource-first form here before the general replacement runs.
var veleroCreateVerbFirstPattern = regexp.MustCompile(`(^|[\s\x60\n])velero\s+create\s+(backup|restore|schedule)\b`)

// replaceVeleroCommandWithOADP performs context-aware replacement of "velero" with "oadp".
// It only replaces "velero" when it's being used as a CLI command, not when referring to
// the Velero project, server, or components.
Expand All @@ -186,6 +194,11 @@ func replaceVeleroCommandWithOADP(text string) string {
// Use "oc" as the CLI prefix since OADP is primarily used on OpenShift
cliPrefix := "oc"

// Normalize verb-first "velero create <resource>" examples to resource-first
// "oc oadp <resource> create" before the general pattern runs, since the actual
// command is always invoked resource-first.
text = veleroCreateVerbFirstPattern.ReplaceAllString(text, "${1}"+cliPrefix+" oadp ${2} create")

// Replace "velero <command>" patterns with "oc/kubectl oadp <command>"
result := veleroCommandPattern.ReplaceAllStringFunc(text, func(match string) string {
// Preserve leading whitespace or backtick
Expand Down
28 changes: 28 additions & 0 deletions cmd/root_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,34 @@ Use velero backup logs to check status`,
}
}

// TestReplaceVeleroWithOADP_ScheduleCreateVerbFirst tests that upstream Velero's
// verb-first "velero create schedule NAME" example text (a known upstream quirk in
// schedule/create.go) is normalized to the resource-first form that actually matches
// how the command is invoked, consistent with "backup create" and "restore create".
func TestReplaceVeleroWithOADP_ScheduleCreateVerbFirst(t *testing.T) {
cmd := &cobra.Command{
Use: "test",
Example: ` # Create a backup every 6 hours.
velero create schedule NAME --schedule="0 */6 * * *"

# Create a backup every 6 hours with the @every notation.
velero create schedule NAME --schedule="@every 6h"`,
}

replaceVeleroWithOADP(cmd)

if strings.Contains(cmd.Example, "velero create schedule") {
t.Errorf("Expected verb-first 'velero create schedule' to be normalized, got: %s", cmd.Example)
}
if strings.Contains(cmd.Example, "velero") {
t.Errorf("Expected 'velero' to be fully replaced, got: %s", cmd.Example)
}
wantCount := strings.Count(cmd.Example, "oc oadp schedule create NAME")
if wantCount != 2 {
t.Errorf("Expected 2 occurrences of 'oc oadp schedule create NAME', got %d\nActual output:\n%s", wantCount, cmd.Example)
}
}
Comment on lines +228 to +250

@kaovilai kaovilai Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And you change this to assert equals (as well) so that we can see the exact replaced statement.

@kaovilai kaovilai Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe followup..

@kaovilai kaovilai Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@coderabbitai create issue.

@coderabbitai coderabbitai Bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="migtools/oadp-cli"
title="Add exact-output assertion for schedule create example normalization"
body=$(cat <<'EOF'
## Summary

Add an exact-output assertion to `TestReplaceVeleroWithOADP_ScheduleCreateVerbFirst` in `cmd/root_test.go`.

## Required change

Keep the existing checks for the removal of `velero create schedule` and the occurrence count. Also compare `cmd.Example` with the complete expected normalized example string. The failure output must show the expected and actual strings.

## Rationale

The occurrence-count check verifies only part of the replacement. An exact-output assertion verifies the complete transformed command text, including both `velero create schedule NAME` replacements and the surrounding content.

## Affected area

- `cmd/root_test.go`
- `TestReplaceVeleroWithOADP_ScheduleCreateVerbFirst`

## Acceptance criteria

- The test declares the complete expected normalized example text.
- The test compares `cmd.Example` to that expected text.
- A test failure reports both the expected and actual values.
- Existing relevant assertions remain valid or are replaced by the exact-output assertion where redundant.

## Backlinks

- Pull request: https://github.com/migtools/oadp-cli/pull/246
- Review comment: https://github.com/migtools/oadp-cli/pull/246#discussion_r3691788318
- Requested by: `@kaovilai`
EOF
)

gh issue create \
  --repo "$repo" \
  --title "$title" \
  --body "$body" \
  --assignee "kaovilai"

Length of output: 2571

@kaovilai, I created and assigned #252: Add exact-output assertion for schedule create example normalization.

🧬 Code Graph Analysis Results

internal/testutil/testutil.go

  • Lines 65–94 — BuildCLIBinary
    Builds the CLI binary from the project root into a temporary directory using go build. Applies a build timeout and fails the test with build output if compilation fails. Used by cmd/root_test.go root-command and namespace tests.

  • Lines 97–122 — RunCommand
    Executes the built binary with the supplied arguments under TestTimeout, captures stdout and stderr, logs command/output details, and returns stdout plus the execution error.

  • Lines 125–140 — TestHelpCommand
    Runs a CLI help command via RunCommand, logs its output, and reports an error for each expected string absent from stdout. Non-zero command exit status is tolerated.

  • Line 34 — TestTimeout
    Defines the 30-second timeout used by RunCommand and related command execution tests.

You are interacting with an AI system.


// TestReplaceVeleroWithOADP_RunFunctionWrapper tests stdout capture and replacement
func TestReplaceVeleroWithOADP_RunFunctionWrapper(t *testing.T) {
outputCaptured := false
Expand Down