Fix test_tools directory

[kdvalin]

User description

Description

This PR sets the test_tools directory to $HOME/test_tools so the common utilities are in a well-known location.

Additionally, this converts the package_tool call to use the convience function, which includes several flags parsed by general_setup.

Before/After Comparison

Before

The test tools would be cloned to wherever the coremark_run script was located.

After

The test tools are now located at $HOME/test_tools

Clerical Stuff

Closes #71
Relates to JIRA: RPOPC-829

---

PR Type

Enhancement, Bug fix

---

Description

• Standardize test_tools location to $HOME/test_tools directory

• Replace hardcoded paths with TOOLS_BIN environment variable

• Convert package_tool call to use convenience function

• Improve path handling with proper quoting for shell safety

---

Diagram Walkthrough

flowchart LR
  A["Hardcoded test_tools paths"] -- "Replace with TOOLS_BIN variable" --> B["Centralized $HOME/test_tools location"]
  C["Direct package_tool call"] -- "Use convenience function" --> D["Simplified package_tool invocation"]
  B --> E["Improved portability and maintainability"]
  D --> E

Loading

File Walkthrough

Relevant files

Enhancement

coremark_run Centralize test_tools path with TOOLS_BIN variable              coremark/coremark_run Added TOOLS_BIN="$HOME/test_tools" variable definition and export at script start Replaced all hardcoded $curdir/test_tools paths with $TOOLS_BIN variable throughout script Updated install_test_tools() to clone to $TOOLS_BIN instead of relative test_tools directory Converted package_tool call to use convenience function without full path prefix Added proper quoting around variable references for shell safety +11/-8

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Untrusted script sourcing Description: The script now sources and executes helper scripts from $HOME/test_tools via $TOOLS_BIN (e.g., source $TOOLS_BIN/general_setup and $TOOLS_BIN/*), which can enable local code execution if an attacker can write to the invoking user's $HOME/test_tools (or if the script is run with elevated privileges and $HOME/test_tools is attacker-controlled), since source runs arbitrary shell code. coremark_run [75-450] Referred Code TOOLS_BIN="$HOME/test_tools" export TOOLS_BIN usage() { echo "Usage $1:" echo " --commit <n>: git commit to use, default is the tag ${coremark_version}" echo " --cpu_add <n>: starting at cpu count of 1, add this number of cpus to each run" echo " --powers_2s: starting at 1, run the number of cpus by powers of 2's" source $TOOLS_BIN/general_setup --usage exit 0 } install_test_tools() { # # Clone the repo that contains the common code and tools # tools_git=https://github.com/redhat-performance/test_tools-wrappers found=0 for arg in "$@"; do ... (clipped 355 lines)
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Unchecked dependencies: Newly added calls to source and tool executables under TOOLS_BIN do not validate file existence/executability or check return codes, which can fail non-gracefully if $HOME/test_tools is missing or incomplete. Referred Code source $TOOLS_BIN/general_setup --usage exit 0 } install_test_tools() { # # Clone the repo that contains the common code and tools # tools_git=https://github.com/redhat-performance/test_tools-wrappers found=0 for arg in "$@"; do if [ $found -eq 1 ]; then tools_git=$arg found=0 fi if [[ $arg == "--tools_git" ]]; then found=1 fi # ... (clipped 249 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Shell word splitting: Several newly added command invocations use unquoted variables/paths (e.g., source $TOOLS_BIN/..., $TOOLS_BIN/gather_data ${curdir}, and ${TOOLS_BIN}/save_results ...) which can enable word-splitting/globbing or argument injection if variables contain spaces or unexpected characters. Referred Code source $TOOLS_BIN/general_setup --usage exit 0 } install_test_tools() { # # Clone the repo that contains the common code and tools # tools_git=https://github.com/redhat-performance/test_tools-wrappers found=0 for arg in "$@"; do if [ $found -eq 1 ]; then tools_git=$arg found=0 fi if [[ $arg == "--tools_git" ]]; then found=1 fi # ... (clipped 248 lines) Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing audit logging: The script performs potentially critical system actions (e.g., git clone, environment setup, and tool execution) without any explicit audit logging of who/what/when/outcome, but it is unclear whether this script is in scope for audit-trail requirements. Referred Code if [ ! -d "$TOOLS_BIN" ]; then git clone $tools_git "$TOOLS_BIN" if [ $? -ne 0 ]; then exit_out "Error: pulling git $tools_git failed." 1 fi fi if [ $show_usage -eq 1 ]; then usage $1 fi } add_time_to_run_log() { log=$1 start_time="$2" end_time="$3" wfile=`mktemp /tmp/coremark_wrk.XXXXX` echo "start_time: $start_time" > $wfile echo "end_time: $end_time" >> $wfile ... (clipped 214 lines) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Unstructured output: The diff does not show any structured logging implementation (and includes commands that may emit unstructured stdout/stderr), but it is unclear whether structured logs are required for this script’s execution context. Referred Code if [ ! -d "$TOOLS_BIN" ]; then git clone $tools_git "$TOOLS_BIN" if [ $? -ne 0 ]; then exit_out "Error: pulling git $tools_git failed." 1 fi fi if [ $show_usage -eq 1 ]; then usage $1 fi } add_time_to_run_log() { log=$1 start_time="$2" end_time="$3" wfile=`mktemp /tmp/coremark_wrk.XXXXX` echo "start_time: $start_time" > $wfile echo "end_time: $end_time" >> $wfile ... (clipped 309 lines) Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Prefix local tools with TOOLS_BIN Prefix the package_tool call with "$TOOLS_BIN/" and quote all variables to ensure the local binary is executed and to handle special characters safely. coremark/coremark_run [446] -package_tool --no_packages $to_no_pkg_install --wrapper_config $curdir/../coremark.json +"$TOOLS_BIN/package_tool" --no_packages "$to_no_pkg_install" --wrapper_config "$curdir/../coremark.json" [Suggestion processed] Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies an inconsistency and potential runtime error, proposing a change that improves both robustness by using an explicit path and safety by quoting variables.	Medium
	Verify script exists before sourcing Add a check to verify that the $TOOLS_BIN/general_setup file exists and is readable before sourcing it to prevent silent failures. coremark/coremark_run [349] +if [ ! -r "$TOOLS_BIN/general_setup" ]; then + exit_out "Error: $TOOLS_BIN/general_setup not found or not readable." 1 +fi source "$TOOLS_BIN/general_setup" "$@" [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: This is a valuable suggestion for improving script robustness by adding an explicit check for the existence of a file before sourcing it, which prevents potential silent failures.	Medium
	Ensure HOME variable is set Use shell parameter expansion "${HOME:?error message}" to ensure the $HOME variable is set before defining TOOLS_BIN, preventing potential path issues. coremark/coremark_run [75-76] -TOOLS_BIN="$HOME/test_tools" +TOOLS_BIN="${HOME:?HOME environment variable is not set}/test_tools" export TOOLS_BIN [Suggestion processed] Suggestion importance[1-10]: 6 __ Why: The suggestion improves script robustness by ensuring the $HOME variable is set, preventing potential errors in minimal execution environments where it might be undefined.	Low
	Quote tool invocation and args Quote the command and its arguments in the $TOOLS_BIN/gather_data invocation to safely handle paths with spaces or special characters. coremark/coremark_run [352] -$TOOLS_BIN/gather_data ${curdir} +"$TOOLS_BIN/gather_data" "$curdir" Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly recommends quoting variables to prevent issues with spaces or special characters, which is a good practice for shell scripting.	Low
More

[github-actions]

This relates to RPOPC-829