OPRUN-4572: Expand OTE docs with more comprehensive details#697
OPRUN-4572: Expand OTE docs with more comprehensive details#697camilamacedo86 wants to merge 1 commit intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@camilamacedo86: This pull request explicitly references no jira issue. DetailsIn response to this: 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. |
WalkthroughExpanded Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 10✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: camilamacedo86 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
openshift/tests-extension/README.md (2)
377-413:⚠️ Potential issue | 🟡 MinorRemove duplicate content.
This entire section duplicates content already covered in the "Presubmit CI Jobs" section (lines 304-376). The YAML example at lines 380-400 is nearly identical to the example at lines 316-335, and the explanation about
include_built_images: true(lines 407-413) duplicates lines 355-357.🧹 Proposed fix: remove redundant lines
-Here is a CI job example: - -```yaml -- as: e2e-aws-techpreview-olmv1-ext - steps: - cluster_profile: aws - env: - FEATURE_SET: TechPreviewNoUpgrade - - # Only enable 'watch-namespaces' monitor to avoid job failures from other default monitors - # in openshift-tests (like apiserver checks, alert summaries, etc). In this job, the selected - # OLMv1 test passed, but the job failed because a default monitor failed. - # - # 'watch-namespaces' is very lightweight and rarely fails, so it's a safe choice. - # There is no way to fully disable all monitors, but we can use this option to reduce noise. - # - # See: ./openshift-tests run --help (option: --monitor) - TEST_ARGS: --monitor=watch-namespaces - - TEST_SUITE: olmv1/all - test: - - ref: openshift-e2e-test - workflow: openshift-e2e-aws -``` - -This uses the `openshift-tests` binary to run OLMv1 tests against a test OpenShift release. - -It works for pull request testing because of this: - -```yaml -releases: - latest: - integration: - include_built_images: true -``` - -More info: https://docs.ci.openshift.org/docs/architecture/ci-operator/#testing-with-an-ephemeral-openshift-release - ## Makefile Commands🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openshift/tests-extension/README.md` around lines 377 - 413, The README contains a duplicated CI job example and explanation that repeats content from the "Presubmit CI Jobs" section; remove the redundant block that starts with the e2e-aws-techpreview-olmv1-ext YAML sample and the follow-up paragraph about releases/include_built_images to avoid duplication, leaving only the original example in the "Presubmit CI Jobs" section and preserving surrounding headings (the duplicate YAML sample and the sentence beginning “This uses the `openshift-tests` binary…” through the `include_built_images: true` snippet should be deleted).
107-118:⚠️ Potential issue | 🟡 MinorRemove or rename duplicate section heading.
The heading "How to Run the Tests Locally" appears twice (lines 107 and 118). This creates confusion and breaks document navigation. Consider either:
- Merging the command table (lines 109-116) into the detailed section starting at line 118, or
- Renaming the first heading to "Quick Reference" or "Command Reference"
📝 Suggested fix: rename first heading
-## How to Run the Tests Locally +## Quick Command Reference | Command | Description |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openshift/tests-extension/README.md` around lines 107 - 118, The README contains a duplicate "How to Run the Tests Locally" heading; remove ambiguity by renaming the first occurrence (the heading immediately above the command table) to "Quick Reference" or "Command Reference" and keep the detailed "How to Run the Tests Locally" section as the primary heading, or alternatively merge the command table under the detailed section; update only the first heading text (the first "How to Run the Tests Locally") so the command table (the block with `make build`, `./bin/olmv1-tests-ext info`, `list`, `run-suite`, `run-test`) is either moved under or referenced by the detailed section to avoid duplicate headings.
🧹 Nitpick comments (1)
openshift/tests-extension/README.md (1)
75-85: Add language specification to fenced code block.The fenced code block lacks a language identifier. Add a language specification (e.g.,
text) to improve rendering and comply with Markdown best practices.📝 Proposed fix
-``` +```text olmv1/extended # All Extended tests ├── extended/releasegate # Extended + ReleaseGate (also in standard suites) └── extended/candidate # Extended without ReleaseGateAs per coding guidelines, the markdownlint tool flagged this: "Fenced code blocks should have a language specified (MD040)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openshift/tests-extension/README.md` around lines 75 - 85, The fenced code block showing the test tree in README.md is missing a language identifier; update the opening fence (the triple backticks before the "olmv1/extended" tree) to include a language spec such as "text" (e.g., change "```" to "```text") so markdownlint MD040 is satisfied and the block renders correctly; ensure only the opening fence is changed and the block content (the tree) is left intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@openshift/tests-extension/README.md`:
- Around line 377-413: The README contains a duplicated CI job example and
explanation that repeats content from the "Presubmit CI Jobs" section; remove
the redundant block that starts with the e2e-aws-techpreview-olmv1-ext YAML
sample and the follow-up paragraph about releases/include_built_images to avoid
duplication, leaving only the original example in the "Presubmit CI Jobs"
section and preserving surrounding headings (the duplicate YAML sample and the
sentence beginning “This uses the `openshift-tests` binary…” through the
`include_built_images: true` snippet should be deleted).
- Around line 107-118: The README contains a duplicate "How to Run the Tests
Locally" heading; remove ambiguity by renaming the first occurrence (the heading
immediately above the command table) to "Quick Reference" or "Command Reference"
and keep the detailed "How to Run the Tests Locally" section as the primary
heading, or alternatively merge the command table under the detailed section;
update only the first heading text (the first "How to Run the Tests Locally") so
the command table (the block with `make build`, `./bin/olmv1-tests-ext info`,
`list`, `run-suite`, `run-test`) is either moved under or referenced by the
detailed section to avoid duplicate headings.
---
Nitpick comments:
In `@openshift/tests-extension/README.md`:
- Around line 75-85: The fenced code block showing the test tree in README.md is
missing a language identifier; update the opening fence (the triple backticks
before the "olmv1/extended" tree) to include a language spec such as "text"
(e.g., change "```" to "```text") so markdownlint MD040 is satisfied and the
block renders correctly; ensure only the opening fence is changed and the block
content (the tree) is left intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b4593940-621d-40f2-9b35-0191d3809d26
📒 Files selected for processing (1)
openshift/tests-extension/README.md
|
@camilamacedo86 CodeRabbit has some suggestions for doc updates you might want to look into. |
camilamacedo86
force-pushed
the
improve-docs
branch
from
84aca0e to
1540e2e
Compare
|
/retest-required |
camilamacedo86
changed the title
|
@camilamacedo86: This pull request references OPRUN-4572 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 story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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. |
|
/test e2e-aws-upgrade-ovn-single-node |
|
@camilamacedo86: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
3 participants
Summary by CodeRabbit