Skip to content

Conversation

@joejstuart
Copy link
Contributor

@joejstuart joejstuart commented Nov 19, 2025

User description

Instead of having to figure out the single test format, this script lists them for you.


PR Type

Enhancement, Tests


Description

  • Add -l/--list flag to display all available features and scenarios

  • Parse feature files to extract and filter features by name or pattern

  • Support filtering by feature and scenario using Feature/Scenario syntax

  • Update help documentation with new listing examples and usage patterns


Diagram Walkthrough

flowchart LR
  A["User runs script<br/>with -l flag"] --> B["parse_args processes<br/>list options"]
  B --> C["list_tests function<br/>parses feature files"]
  C --> D["Extract features<br/>and scenarios"]
  D --> E["Filter by patterns<br/>if provided"]
  E --> F["Display formatted<br/>results"]
Loading

File Walkthrough

Relevant files
Enhancement
run-acceptance-tests.sh
Add feature and scenario listing functionality                     

hack/macos/run-acceptance-tests.sh

  • Added new list_tests() function that parses .feature files and
    displays available features and scenarios with optional pattern
    filtering
  • Implemented -l/--list command-line option to trigger listing mode with
    support for feature and scenario pattern matching
  • Extended parse_args() to handle list-specific arguments including
    feature/scenario pattern parsing using Feature/Scenario syntax
  • Updated show_usage() help text with new -l/--list option documentation
    and realistic feature-based examples
  • Modified main() to detect listing mode early and skip unnecessary
    setup steps
+163/-7 

Instead of having to figure out the single test
format, this script lists them for you.
@qodo-code-review
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Unescaped grep pattern

Description: User-controlled pattern is passed to grep without being quoted or escaped, enabling
potential regular-expression injection or unintended matches (e.g., a pattern like ".*" or
"^[^]" could alter results or cause errors) and, in extreme debug/alias contexts, could
behave unpredictably.
run-acceptance-tests.sh [156-160]

Referred Code
if [ -n "$feature_pattern" ]; then
    feature_files=$(find features -name "*.feature" -type f | grep -i "$feature_pattern" | sort)
else
    feature_files=$(find features -name "*.feature" -type f | sort)
fi
Untrusted output escapes

Description: Feature names from files are interpolated into echo -e with ANSI color variables; if a
feature name contains backslash escape sequences, echo -e may interpret them and produce
terminal control effects or misleading output; use printf '%s' to avoid escape
interpretation.
run-acceptance-tests.sh [192-197]

Referred Code
if [ -z "$feature_pattern" ] || echo "$feature_name" | grep -qi "$feature_pattern"; then
    show_this_feature=true
    echo
    echo -e "${GREEN}Feature:${NC} $feature_name"
    echo -e "  ${CYAN}File:${NC} $current_file"
fi
Unsanitized program name

Description: Help output echoes "$0" directly, which can be attacker-controlled if the script is
invoked via a crafted path, potentially causing confusing or misleading output; prefer a
fixed program name or sanitize/basename "$0".
run-acceptance-tests.sh [231-235]

Referred Code
print_status "To run a specific feature/scenario, use:"
print_status "  $0 'FeatureName'                    # Run all scenarios in a feature"
print_status "  $0 'FeatureName/ScenarioName'        # Run a specific scenario"
print_status "  $0 'FeaturePattern'                  # Run features matching pattern"
print_status "  $0 'FeaturePattern/ScenarioPattern' # Run scenarios matching pattern"
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: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

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 Auditing: The newly added listing functionality performs user-invoked actions without any explicit
audit logging of who invoked it or the outcome, which may be acceptable for a local dev
script but cannot be confirmed from the diff.

Referred Code
# List all available tests from feature files
list_tests() {
    local feature_pattern="${1:-}"
    local scenario_pattern="${2:-}"

    print_status "Listing available features and scenarios..."

    # Check if features directory exists
    if [ ! -d "features" ]; then
        print_error "features directory not found in $(pwd)"
        exit 1
    fi

    local feature_count=0
    local scenario_count=0
    local current_feature=""
    local current_file=""

    # Find all .feature files
    local feature_files
    if [ -n "$feature_pattern" ]; then


 ... (clipped 80 lines)

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:
Edge Cases: The listing logic relies on grep/regex parsing and may mis-handle features without
descriptions or unusual whitespace; error handling largely prints and exits but
completeness across all edge cases (e.g., unreadable files) is uncertain.

Referred Code
# Parse each feature file
while IFS= read -r feature_file; do
    if [ ! -f "$feature_file" ]; then
        continue
    fi

    current_file=$(basename "$feature_file")
    local feature_name=""
    local feature_description=""
    local show_this_feature=false
    local in_feature=false
    local next_line_is_description=false

    # Read the feature file line by line
    while IFS= read -r line || [ -n "$line" ]; do
        # Check for Feature: line
        if [[ "$line" =~ ^Feature:[[:space:]]*(.+) ]]; then
            feature_name="${BASH_REMATCH[1]}"
            feature_count=$((feature_count + 1))
            in_feature=true
            next_line_is_description=true


 ... (clipped 47 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:
Input Validation: User-supplied patterns are passed to grep without clear sanitization which could lead to
unintended regex behavior or errors, though risk is limited in a local test script; needs
confirmation of intended regex vs literal matching.

Referred Code
if [ -n "$feature_pattern" ]; then
    feature_files=$(find features -name "*.feature" -type f | grep -i "$feature_pattern" | sort)
else
    feature_files=$(find features -name "*.feature" -type f | sort)
fi

if [ -z "$feature_files" ]; then
    print_warning "No feature files found matching pattern: ${feature_pattern:-'*'}"
    return 0
fi

echo

# Parse each feature file
while IFS= read -r feature_file; do
    if [ ! -f "$feature_file" ]; then
        continue
    fi

    current_file=$(basename "$feature_file")
    local feature_name=""


 ... (clipped 39 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
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Leverage the test runner for listing

Instead of manually parsing .feature files with a complex bash function, use the
godog test runner's built-in capabilities (like --dry-run) to list tests. This
approach is simpler, more robust, and guarantees the list is accurate.

Examples:

hack/macos/run-acceptance-tests.sh [137-236]
list_tests() {
    local feature_pattern="${1:-}"
    local scenario_pattern="${2:-}"
    
    print_status "Listing available features and scenarios..."
    
    # Check if features directory exists
    if [ ! -d "features" ]; then
        print_error "features directory not found in $(pwd)"
        exit 1

 ... (clipped 90 lines)

Solution Walkthrough:

Before:

list_tests() {
    # Find all .feature files
    feature_files=$(find features -name "*.feature" ...)

    # Manually parse each file line by line
    while IFS= read -r feature_file; do
        while IFS= read -r line; do
            # Use regex to find "Feature:"
            if [[ "$line" =~ ^Feature:[[:space:]]*(.+) ]]; then
                # ... logic to extract and print feature
            fi
            # Use regex to find "Scenario:"
            elif [[ "$line" =~ ^[[:space:]]*Scenario:[[:space:]]*(.+) ]]; then
                # ... logic to extract and print scenario
            fi
            # ... more complex state management and parsing logic
        done < "$feature_file"
    done <<< "$feature_files"
}

After:

list_tests() {
    # Delegate test listing to the actual test runner (e.g., godog)
    # This avoids manual parsing and ensures accuracy.
    # The exact command might vary.
    print_status "Listing available tests using the test runner..."

    # Example using a hypothetical dry-run format
    godog --dry-run --format=pretty | \
        grep -E "^\s*(Feature:|Scenario:)"
    
    # Filtering can be applied to the output or passed as arguments
    # to the test runner if supported.
}
Suggestion importance[1-10]: 9

__

Why: This is a major design improvement that replaces a complex, brittle custom parser with a robust, maintainable approach, significantly improving the quality and reliability of the new feature.

High
Possible issue
Simplify and correct feature filtering logic

To fix incorrect feature filtering, remove the initial grep on file paths and
rely only on the subsequent filtering of the feature name from the file's
content.

hack/macos/run-acceptance-tests.sh [154-160]

 # Find all .feature files
 local feature_files
-if [ -n "$feature_pattern" ]; then
-    feature_files=$(find features -name "*.feature" -type f | grep -i "$feature_pattern" | sort)
-else
-    feature_files=$(find features -name "*.feature" -type f | sort)
-fi
+feature_files=$(find features -name "*.feature" -type f | sort)
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a logic bug where filtering by filename can incorrectly exclude feature files before their content is checked, leading to missed results.

Medium
Correctly handle multi-line feature descriptions

Update the parsing logic to handle multi-line Gherkin feature descriptions by
accumulating all description lines until a keyword is found, ensuring the full
description is displayed.

hack/macos/run-acceptance-tests.sh [198-207]

 # Check for description line (first non-empty line after Feature:)
 elif [ "$next_line_is_description" = true ] && [ "$in_feature" = true ]; then
     if [[ ! "$line" =~ ^[[:space:]]*$ ]] && [[ ! "$line" =~ ^[[:space:]]*(Background|Scenario|Given|When|Then|And|But) ]]; then
+        if [ -z "$feature_description" ]; then
+            feature_description="$line"
+        else
+            feature_description+=$'\n'"  $line"
+        fi
+    elif [[ -n "$feature_description" ]]; then
         if [ "$show_this_feature" = true ]; then
-            echo -e "  ${CYAN}Description:${NC} $line"
+            echo -e "  ${CYAN}Description:${NC}${feature_description}"
         fi
         next_line_is_description=false
     fi
 # Check for Scenario: line
 elif [[ "$line" =~ ^[[:space:]]*Scenario:[[:space:]]*(.+) ]] && [ "$in_feature" = true ]; then
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out that the script fails to handle multi-line Gherkin feature descriptions and proposes a valid fix to accumulate and display the full description.

Low
General
Avoid duplicating argument parsing logic

Remove the redundant check for the --list flag from the main function, as this
logic is already handled within parse_args, to simplify the script's entry
point.

hack/macos/run-acceptance-tests.sh [445-463]

 # Main execution
 main() {
-+    # Check if we're just listing tests (quick check without full setup)
-+    for arg in "$@"; do
-+        if [[ "$arg" == "-l" ]] || [[ "$arg" == "--list" ]]; then
-+            echo "=========================================="
-+            echo "  Listing Acceptance Tests"
-+            echo "=========================================="
-+            echo
-+            
-+            # Skip setup for listing - it's not needed
-+            parse_args "$@"
-+            exit 0
-+        fi
-+    done
-+    
+    # The 'parse_args' function will handle all argument parsing,
+    # including the --list flag which will cause the script to exit
+    # after listing tests, skipping the setup below.
+
     echo "=========================================="
     echo "  Running Acceptance Tests"
     echo "=========================================="
     echo
 ...

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies redundant logic for handling the --list flag in the main function and proposes a good refactoring to improve code clarity and maintainability.

Low
  • More

Copy link
Contributor

@robnester-rh robnester-rh left a comment

Choose a reason for hiding this comment

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

No issues, but I was curious if you'd looked at the suggestions from qodo?

@joejstuart
Copy link
Contributor Author

No issues, but I was curious if you'd looked at the suggestions from qodo?

Just tried the first suggestion

❯ godog --dry-run --format=pretty | grep -E "^\s*(Feature:|Scenario:)"
Error: unknown flag: --dry-run

@joejstuart joejstuart merged commit 80e13a9 into conforma:main Nov 21, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants