Skip to content

docs: update README#57

Open
sayalibhavsar wants to merge 1 commit intomainfrom
docs/update-readme
Open

docs: update README#57
sayalibhavsar wants to merge 1 commit intomainfrom
docs/update-readme

Conversation

@sayalibhavsar
Copy link
Copy Markdown
Collaborator

@sayalibhavsar sayalibhavsar commented Apr 21, 2026

Description

changes made to passmark-wrapper documentation

Before/After Comparison

Changes include a template followed across all wrapper

Solves issue: #56
Relates to JIRA: RPOPC-939

@github-actions
Copy link
Copy Markdown

This relates to RPOPC-940

@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Comprehensive PassMark wrapper documentation overhaul

📝 Documentation

Grey Divider

Walkthroughs

Description
• Comprehensive README rewrite with detailed workflow documentation
• Added command-line options reference and usage examples
• Included complete benchmark test categories with descriptions
• Added architecture support details and troubleshooting guide
• Documented output files, dependencies, and return codes
Diagram
flowchart LR
  A["Old README<br/>Basic description"] -->|"Expand and<br/>restructure"| B["New README<br/>Complete guide"]
  B --> C["Command options<br/>& usage"]
  B --> D["Workflow steps<br/>1-9"]
  B --> E["Test categories<br/>CPU/Memory/Crypto"]
  B --> F["Examples &<br/>troubleshooting"]
Loading

Grey Divider

File Changes

1. README.md 📝 Documentation +242/-31

Complete documentation restructure with detailed workflow guide

• Expanded from 42 to 253 lines with comprehensive documentation structure
• Added detailed 9-step workflow explanation covering environment setup through result verification
• Included command-line options reference with PassMark-specific and general test_tools options
• Added benchmark test categories with tables for CPU, cryptography, and memory tests
• Documented output files, dependencies by OS, return codes, and architecture-specific handling
• Included practical examples covering basic runs, multiple iterations, PCP monitoring, and custom
 repositories
• Added troubleshooting section, performance tips, and notes on libncurses compatibility

README.md


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented Apr 21, 2026

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Broken CPU flags documented 🐞 Bug ≡ Correctness
Description
README documents --cpu_add/--powers_2 as supported, but passmark_run does not handle these flags and
will treat them as unknown options and exit via usage. Users following the README will immediately
fail when trying these advertised options.
Code

README.md[R20-22]

+PassMark Options:
+  --cpu_add <n>: Add n CPUs each iteration until max CPUs reached.
+  --powers_2: Starting from 1 CPU, keep doubling the CPUs until max CPUs reached.
Evidence
README claims the wrapper supports --cpu_add and --powers_2. In passmark_run, getopt includes these
long options, but the argument-processing case statement only handles --usage/-h/-- and sends any
other option (including --cpu_add/--powers_2) to the default "option not found" path, which calls
usage and exits.

README.md[17-23]
passmark/passmark_run[397-433]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
README advertises `--cpu_add` and `--powers_2`, but `passmark_run` exits with usage when either flag is provided because they are never handled in the option parsing loop.

## Issue Context
`getopt` is configured to recognize these flags, but the `case` statement only handles `--usage` and `-h`. This makes the documented flags unusable and breaks user workflows.

## Fix Focus Areas
- passmark/passmark_run[397-433]
- README.md[17-23]

## Expected fix
Choose one:
1) **Implement** `--cpu_add` and `--powers_2` (add `case` branches, store values, and apply them to the run logic), and keep README as-is.
2) **Remove/adjust documentation** (and ideally also remove them from `getopt`/usage output) if the wrapper does not actually support these behaviors.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. tools_git option miscategorized 🐞 Bug ⚙ Maintainability
Description
README lists --tools_git under “General test_tools options”, but passmark_run parses it before
cloning/sourcing test_tools and it is not shown by passmark_run --usage. This misleads users about
where the option comes from and how discoverable it is.
Code

README.md[R24-36]

+General test_tools options:
+  --home_parent <value>: Parent home directory. If not set, defaults to current working directory.
+  --host_config <value>: Host configuration name, defaults to current hostname.
+  --iterations <value>: Number of times to run the test, defaults to 1.
+  --run_user: User that is actually running the test on the test system. Defaults to current user.
+  --sys_type: Type of system working with (aws, azure, hostname). Defaults to hostname.
+  --sysname: Name of the system running, used in determining config files. Defaults to hostname.
+  --tuned_setting: Used in naming the results directory. For RHEL, defaults to current active tuned profile.
+      For non-RHEL systems, defaults to 'none'.
+  --use_pcp: Enable Performance Co-Pilot monitoring during test execution.
+  --tools_git <value>: Git repo to retrieve the required tools from.
+      Default: https://github.com/redhat-performance/test_tools-wrappers
+  --usage: Display this usage message.
Evidence
The README groups --tools_git with general test_tools options. In the script, --tools_git is parsed
in install_tools before test_tools is available, and the script’s usage() output does not mention
--tools_git (it only prints passmark-specific flags then delegates to general_setup --usage).

README.md[24-36]
passmark/passmark_run[108-115]
passmark/passmark_run[200-243]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
README categorizes `--tools_git` as a “General test_tools option”, but in `passmark_run` it is handled as a wrapper bootstrap option (parsed before cloning/sourcing test_tools) and it is not displayed in the wrapper’s `--usage` output.

## Issue Context
This is documentation/UX drift: users looking at `./passmark_run --usage` won’t see `--tools_git`, and the README implies it’s part of general test_tools options.

## Fix Focus Areas
- README.md[24-36]
- passmark/passmark_run[108-115]
- passmark/passmark_run[200-243]

## Expected fix
Either:
- Move `--tools_git` out of the “General test_tools options” list into a “Wrapper bootstrap options” section (or similar), **and/or**
- Update `usage()` in `passmark_run` to explicitly print `--tools_git <value>` so it appears in `--usage` output.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@github-actions
Copy link
Copy Markdown

This relates to RPOPC-939

Comment thread README.md
Comment on lines +20 to +22
PassMark Options:
--cpu_add <n>: Add n CPUs each iteration until max CPUs reached.
--powers_2: Starting from 1 CPU, keep doubling the CPUs until max CPUs reached.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Broken cpu flags documented 🐞 Bug ≡ Correctness

README documents --cpu_add/--powers_2 as supported, but passmark_run does not handle these flags and
will treat them as unknown options and exit via usage. Users following the README will immediately
fail when trying these advertised options.
Agent Prompt
## Issue description
README advertises `--cpu_add` and `--powers_2`, but `passmark_run` exits with usage when either flag is provided because they are never handled in the option parsing loop.

## Issue Context
`getopt` is configured to recognize these flags, but the `case` statement only handles `--usage` and `-h`. This makes the documented flags unusable and breaks user workflows.

## Fix Focus Areas
- passmark/passmark_run[397-433]
- README.md[17-23]

## Expected fix
Choose one:
1) **Implement** `--cpu_add` and `--powers_2` (add `case` branches, store values, and apply them to the run logic), and keep README as-is.
2) **Remove/adjust documentation** (and ideally also remove them from `getopt`/usage output) if the wrapper does not actually support these behaviors.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@github-actions
Copy link
Copy Markdown

This relates to RPOPC-943

@sayalibhavsar sayalibhavsar requested a review from kdvalin April 21, 2026 15:15
@sayalibhavsar sayalibhavsar self-assigned this Apr 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant