MGMT-24400: Document TestVersion API and usage guidance

[bluesort]

• Add docs/dev/test-versions.md documenting the TestVersion() fluent API (constraints, architecture filtering, terminal methods) with usage patterns for subsystem tests
• Add a section to docs/dev/testing.md clarifying when to use TestVersion() (subsystem) vs hardcoded strings (unit tests)
• Update the unit test writing skill to flag TestVersion() usage outside subsystem/ as an anti-pattern, with correct alternatives (hardcoded strings or common.TestDefaultConfig)

List all the issues related to this PR

• New Feature
• Enhancement
• Bug fix
• Tests
• Documentation
• CI/CD

What environments does this code impact?

• Automation (CI, tools, etc)
• Cloud
• Operator Managed Deployments
• None

How was this code tested?

• assisted-test-infra environment
• dev-scripts environment
• Reviewer's test appreciated
• Waiting for CI to do a full test run
• Manual (Elaborate on how it was tested)
• No tests needed

Checklist

• Title and description added to both, commit and PR.
• Relevant issues have been associated (see CONTRIBUTING guide)
• This change does not require a documentation update (docstring, docs, README, etc)
• Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

• Are the title and description (in both PR and commit) meaningful and clear?
• Is there a bug required (and linked) for this change?
• Should this PR be backported?

Summary by CodeRabbit

• Documentation
  • Clarified assisted-service unit test writing guidance to emphasize non-subsystem test OCP version handling, including when to avoid TestVersion() and use hardcoded values or defaults
  • Added developer documentation covering TestVersion() behavior for subsystem tests, including supported fluent APIs, constraint selection semantics, and release/artifact lookup rules
  • Updated general testing documentation with a new “Writing Tests with OCP Versions” section detailing recommended OCP version reference patterns per test type

[openshift-ci-robot]

@bluesort: This pull request references MGMT-24400 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the sub-task to target the "5.0.0" version, but no target version was set.

Details

In response to this:

• Add docs/dev/test-versions.md documenting the TestVersion() fluent API (constraints, architecture filtering, terminal methods) with usage patterns for subsystem tests
• Add a section to docs/dev/testing.md clarifying when to use TestVersion() (subsystem) vs hardcoded strings (unit tests)
• Update the unit test writing skill to flag TestVersion() usage outside subsystem/ as an anti-pattern, with correct alternatives (hardcoded strings or common.TestDefaultConfig)

List all the issues related to this PR

• New Feature
• Enhancement
• Bug fix
• Tests
• Documentation
• CI/CD

What environments does this code impact?

• Automation (CI, tools, etc)
• Cloud
• Operator Managed Deployments
• None

How was this code tested?

• assisted-test-infra environment
• dev-scripts environment
• Reviewer's test appreciated
• Waiting for CI to do a full test run
• Manual (Elaborate on how it was tested)
• No tests needed

Checklist

• Title and description added to both, commit and PR.
• Relevant issues have been associated (see CONTRIBUTING guide)
• This change does not require a documentation update (docstring, docs, README, etc)
• Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

• Are the title and description (in both PR and commit) meaningful and clear?
• Is there a bug required (and linked) for this change?
• Should this PR be backported?

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 725b67fe-c09d-4c7d-a5b8-66937bd32c31

📥 Commits

Reviewing files that changed from the base of the PR and between c884d32 and eb3d45c.

📒 Files selected for processing (3)

• .cursor/skills/assisted-service-writing-unit-tests/SKILL.md
• docs/dev/test-versions.md
• docs/dev/testing.md

✅ Files skipped from review due to trivial changes (3)

• docs/dev/testing.md
• docs/dev/test-versions.md
• .cursor/skills/assisted-service-writing-unit-tests/SKILL.md

---

Walkthrough

This PR updates documentation to clarify OCP version handling in assisted-service tests. Subsystem tests must reference OCP versions via TestVersion() to avoid breakage during version pruning, while unit tests use hardcoded version strings or TestDefaultConfig values. The changes include updated cursor skill guidance, a new comprehensive TestVersion() API reference, and a high-level testing documentation section.

Changes

OCP Version Testing Documentation

Layer / File(s)	Summary
Testing guidance framework docs/dev/testing.md	New section establishes the testing rule: subsystem tests must use common.TestVersion() for OCP version references, while unit tests use hardcoded strings, except TestDefaultConfig which uses TestVersion().Latest().
Non-subsystem test writing guidance .cursor/skills/assisted-service-writing-unit-tests/SKILL.md	Cursor skill scope is clarified to non-subsystem tests. Front-matter description, "Scope" section with subsystem caveat, new "OCP Versions" section forbidding TestVersion() with hardcoded and TestDefaultConfig examples, and Red Flags/Common Mistakes/Before Submitting checklist items enforce the restriction.
TestVersion() fluent API reference docs/dev/test-versions.md	New comprehensive guide documents the TestVersion() fluent builder for subsystem tests, including constraint chaining (GreaterThan*, LessThan*, Exact), architecture filtering, terminal method semantics (Version vs TryVersion), URL helpers, and make generate regeneration workflow.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 15

✅ Passed checks (15 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main purpose of the PR, which is documenting the TestVersion API and its usage guidance across multiple documentation files.
Description check	✅ Passed	The PR description provides clear details about all three documentation changes, properly marks the Documentation checkbox, and includes all relevant template sections with appropriate selections.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names	✅ Passed	PR is documentation-only (3 .md files), contains no executable Ginkgo test code. The check applies to test code, not documentation.
Test Structure And Quality	✅ Passed	This is a documentation-only PR with no Ginkgo test code files (.go) being added or modified. The check is not applicable to documentation changes or code examples in markdown files.
Microshift Test Compatibility	✅ Passed	No new Ginkgo e2e tests are added in this PR. The changes are documentation-only (3 markdown files): test-versions.md (new), testing.md (updated), and SKILL.md (updated). The check is not applicable.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	This PR contains only documentation changes (docs/dev/test-versions.md, docs/dev/testing.md, SKILL.md) with no new Ginkgo e2e tests being added, making the SNO compatibility check not applicable.
Topology-Aware Scheduling Compatibility	✅ Passed	PR contains documentation-only changes (test-versions.md, testing.md, SKILL.md). No deployment manifests, operator code, or controllers are added/modified, so the topology-aware scheduling check do...
Ote Binary Stdout Contract	✅ Passed	PR contains only documentation files (.md files) with no source code changes. OTE Binary Stdout Contract check applies to executable code (main(), init(), BeforeSuite(), etc.), which is not present...
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	Check is not applicable: PR contains only documentation changes (3 markdown files). No Ginkgo e2e tests or test code was added, modified, or is subject to IPv6/disconnected network constraints.
No-Weak-Crypto	✅ Passed	PR contains only documentation changes (3 MD files). No cryptographic code, weak crypto algorithms (MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB), custom crypto implementations, or non-constant-time co...
Container-Privileges	✅ Passed	PR is documentation-only; contains no container manifests, K8s configs, or Dockerfiles that could have privilege escalation settings.
No-Sensitive-Data-In-Logs	✅ Passed	Documentation-only PR with no logging code, no sensitive data, and no hardcoded credentials in URLs or examples. Check not applicable to documentation changes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bluesort

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [bluesort]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

🧹 Nitpick comments (2)

.cursor/skills/assisted-service-writing-unit-tests/SKILL.md (1)

23-23: ⚡ Quick win

Standardize on common.TestVersion() wording across this skill.

These lines mix TestVersion() and common.TestVersion(). Use the fully-qualified form everywhere to keep rule-checking and copy/paste guidance unambiguous.

Suggested edits

-| OCP version in tests | Use hardcoded strings or `common.TestDefaultConfig` — do not use `TestVersion()` |
+| OCP version in tests | Use hardcoded strings or `common.TestDefaultConfig` — do not use `common.TestVersion()` |

-## OCP Versions: Use Hardcoded Strings, Not TestVersion()
+## OCP Versions: Use Hardcoded Strings, Not common.TestVersion()

-| Using `TestVersion()` | Wrong abstraction for non-subsystem tests | Use hardcoded strings or `common.TestDefaultConfig` |
+| Using `common.TestVersion()` | Wrong abstraction for non-subsystem tests | Use hardcoded strings or `common.TestDefaultConfig` |

-- [ ] No `TestVersion()` usage — use hardcoded strings or `common.TestDefaultConfig`
+- [ ] No `common.TestVersion()` usage — use hardcoded strings or `common.TestDefaultConfig`

Also applies to: 138-140, 179-179, 188-188

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.cursor/skills/assisted-service-writing-unit-tests/SKILL.md at line 23,
Replace all occurrences of the unqualified symbol TestVersion() with the
fully-qualified common.TestVersion() in the SKILL.md content so wording is
consistent; search for every instance of "TestVersion()" (including the ones
noted near the OCP version guidance and the other occurrences) and update them
to "common.TestVersion()" to standardize examples and rule-checking guidance.

docs/dev/testing.md (1)

88-88: ⚡ Quick win

Use fully-qualified API in this exception example.

This line should use common.TestVersion().Latest() for consistency with the rest of the docs and to avoid ambiguous copy/paste usage.

Suggested edit

-**Unit tests** may use hardcoded version strings since versions are not pulled from real sources. The one exception is `TestDefaultConfig` in `internal/common/test_configuration.go`, which uses `TestVersion().Latest()` to conveniently track the latest available version.
+**Unit tests** may use hardcoded version strings since versions are not pulled from real sources. The one exception is `TestDefaultConfig` in `internal/common/test_configuration.go`, which uses `common.TestVersion().Latest()` to conveniently track the latest available version.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/dev/testing.md` at line 88, Update the example text to use the
fully-qualified API by replacing the ambiguous call TestVersion().Latest() with
common.TestVersion().Latest() in the docs; specifically locate the sentence
describing TestDefaultConfig in internal/common/test_configuration.go and change
the example usage to common.TestVersion().Latest() so it matches the rest of the
docs and avoids copy/paste ambiguity.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In @.cursor/skills/assisted-service-writing-unit-tests/SKILL.md:
- Line 23: Replace all occurrences of the unqualified symbol TestVersion() with
the fully-qualified common.TestVersion() in the SKILL.md content so wording is
consistent; search for every instance of "TestVersion()" (including the ones
noted near the OCP version guidance and the other occurrences) and update them
to "common.TestVersion()" to standardize examples and rule-checking guidance.

In `@docs/dev/testing.md`:
- Line 88: Update the example text to use the fully-qualified API by replacing
the ambiguous call TestVersion().Latest() with common.TestVersion().Latest() in
the docs; specifically locate the sentence describing TestDefaultConfig in
internal/common/test_configuration.go and change the example usage to
common.TestVersion().Latest() so it matches the rest of the docs and avoids
copy/paste ambiguity.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2374a784-a650-4e60-aead-8acbccc7b2f9

📥 Commits

Reviewing files that changed from the base of the PR and between 026a5c7 and c884d32.

📒 Files selected for processing (3)

• .cursor/skills/assisted-service-writing-unit-tests/SKILL.md
• docs/dev/test-versions.md
• docs/dev/testing.md

[openshift-ci]

@bluesort: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.