From 0e8dbdbac1e8400c3c9b8ff25540bbe504b04b6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20Sj=C3=B6blom?= Date: Mon, 2 Mar 2026 08:39:47 +0100 Subject: [PATCH 1/3] Adding Valgrind workflow for GitHub Actions --- .dockerignore | 32 +++++++- .github/workflows/pr.yml | 2 + .github/workflows/valgrind.yml | 134 +++++++++++++++++++++++++++++++++ Dockerfile.valgrind | 48 ++++++++++++ README.md | 2 +- test/test_about_version.py | 84 +++++++++++++++++++++ 6 files changed, 300 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/valgrind.yml create mode 100644 Dockerfile.valgrind create mode 100644 test/test_about_version.py diff --git a/.dockerignore b/.dockerignore index 796b96d..8dfae76 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1 +1,31 @@ -/build +# Build directories +build +build/ +build/** +bin/ +lib/ + +# Python +test/venv/ +__pycache__/ +*.pyc +*.pyo +*.pyd +.pytest_cache/ + +# Valgrind logs +valgrind_logs/ + +# Git +.git/ + +# IDE +.vscode/ +.idea/ +*.swp +*.swo +*~ + +# OS +.DS_Store +Thumbs.db diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 404f931..eddf54d 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -12,3 +12,5 @@ jobs: test: uses: ./.github/workflows/test.yml needs: build + valgrind: + uses: ./.github/workflows/valgrind.yml diff --git a/.github/workflows/valgrind.yml b/.github/workflows/valgrind.yml new file mode 100644 index 0000000..a37ee21 --- /dev/null +++ b/.github/workflows/valgrind.yml @@ -0,0 +1,134 @@ +name: Valgrind Memory Leak Check + +on: + pull_request: + branches: [ main ] + workflow_dispatch: + +permissions: + contents: read + +jobs: + valgrind: + runs-on: ubuntu-24.04 + # This job is allowed to fail without blocking PR merges + continue-on-error: true + + steps: + - name: Check out repository code + uses: actions/checkout@v6 + with: + submodules: true + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v3 + + - name: Build Valgrind Docker image + run: | + docker build \ + --build-arg USER_ID=$(id -u) \ + --build-arg GROUP_ID=$(id -g) \ + -t mpqcli-valgrind \ + -f Dockerfile.valgrind \ + . + + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: '3.11' + + - name: Install test dependencies + run: | + python -m pip install --upgrade pip + pip install -r test/requirements.txt + + - name: Run Valgrind on test suite + id: valgrind_tests + run: | + # Create output directory + mkdir -p valgrind_logs + + # Run tests with Valgrind using Docker + docker run --rm \ + -v $(pwd)/test:/mpqcli/test:ro \ + -v $(pwd)/valgrind_logs:/mpqcli/valgrind_logs:rw \ + -e MPQCLI_BINARY=/mpqcli/build/bin/mpqcli \ + mpqcli-valgrind \ + bash -c "cd /mpqcli && python3 -m pytest test -v --tb=short || true" + + echo "Test execution completed" + + - name: Analyze Valgrind results + id: analyze + run: | + # Count total errors across all log files + total_errors=0 + total_leaks=0 + + if [ -d "valgrind_logs" ] && [ "$(ls -A valgrind_logs)" ]; then + for log in valgrind_logs/*.log; do + if [ -f "$log" ]; then + errors=$(grep -c "ERROR SUMMARY:" "$log" || echo "0") + leaks=$(grep -c "definitely lost:" "$log" || echo "0") + total_errors=$((total_errors + errors)) + total_leaks=$((total_leaks + leaks)) + fi + done + fi + + echo "Total error summaries found: $total_errors" + echo "Total leak reports found: $total_leaks" + + # Create summary + echo "## Valgrind Memory Leak Analysis" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "### Subcommands Tested" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "**Tested via pytest suite:**" >> $GITHUB_STEP_SUMMARY + echo "- \`create\` - Create MPQ archives" >> $GITHUB_STEP_SUMMARY + echo "- \`add\` - Add files to archives" >> $GITHUB_STEP_SUMMARY + echo "- \`remove\` - Remove files from archives" >> $GITHUB_STEP_SUMMARY + echo "- \`list\` - List archive contents" >> $GITHUB_STEP_SUMMARY + echo "- \`extract\` - Extract files from archives" >> $GITHUB_STEP_SUMMARY + echo "- \`read\` - Read files from archives" >> $GITHUB_STEP_SUMMARY + echo "- \`verify\` - Verify archive signatures" >> $GITHUB_STEP_SUMMARY + echo "- \`info\` - Display archive information" >> $GITHUB_STEP_SUMMARY + echo "- \`about\` - Display mpqcli information" >> $GITHUB_STEP_SUMMARY + echo "- \`version\` - Display mpqcli version" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "### Results" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + + if [ "$total_errors" -eq 0 ] && [ "$total_leaks" -eq 0 ]; then + echo "✅ **No memory leaks detected in any subcommand!**" >> $GITHUB_STEP_SUMMARY + echo "status=success" >> $GITHUB_OUTPUT + else + echo "⚠️ **Potential memory issues detected**" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "- Error summaries: $total_errors" >> $GITHUB_STEP_SUMMARY + echo "- Leak reports: $total_leaks" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "Please review the Valgrind logs for details." >> $GITHUB_STEP_SUMMARY + echo "status=warning" >> $GITHUB_OUTPUT + fi + + - name: Upload Valgrind logs + if: always() + uses: actions/upload-artifact@v4 + with: + name: valgrind-logs + path: | + valgrind_logs/ + retention-days: 30 + + - name: Comment on PR (if leaks found) + if: github.event_name == 'pull_request' && steps.analyze.outputs.status == 'warning' + uses: actions/github-script@v7 + with: + script: | + github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + body: '⚠️ **Valgrind detected potential memory leaks**\n\nPlease review the [Valgrind logs](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) for details.\n\n*Note: This check is informational and does not block merging.*' + }) diff --git a/Dockerfile.valgrind b/Dockerfile.valgrind new file mode 100644 index 0000000..4c26315 --- /dev/null +++ b/Dockerfile.valgrind @@ -0,0 +1,48 @@ +FROM ubuntu:22.04 + +# Install build dependencies and Valgrind +RUN apt-get update +RUN apt-get install -y \ + build-essential \ + cmake \ + git \ + valgrind \ + python3 \ + python3-pip \ + python3-venv \ + sudo +RUN rm -rf /var/lib/apt/lists/* + +# Create a user with the same UID/GID as the host user (passed as build args) +ARG USER_ID=1000 +ARG GROUP_ID=1000 +RUN groupadd -g ${GROUP_ID} mpquser || true && \ + useradd -m -u ${USER_ID} -g ${GROUP_ID} -s /bin/bash mpquser && \ + echo "mpquser ALL=(ALL) NOPASSWD:ALL" >> /etc/sudoers + +WORKDIR /mpqcli + +# Copy the entire project +COPY . . + +# Remove any existing build directory and fix ownership +RUN rm -rf build && \ + chown -R mpquser:mpquser /mpqcli + +# Switch to non-root user +USER mpquser + +# Build with debug symbols (needed by Valgrind) +RUN cmake -B build \ + -DCMAKE_BUILD_TYPE=Debug \ + -DBUILD_MPQCLI=ON + +RUN cmake --build build + +# Set up Python test environment +RUN python3 -m venv test/venv && \ + . test/venv/bin/activate +RUN pip3 install -r test/requirements.txt + +# Default command runs Valgrind on the binary +CMD ["valgrind", "--leak-check=full", "--show-leak-kinds=all", "--track-origins=yes", "--verbose", "./build/bin/mpqcli", "version"] diff --git a/README.md b/README.md index bee39e5..7655ff0 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # mpqcli -![Build Status](https://img.shields.io/github/actions/workflow/status/TheGrayDot/mpqcli/tag.yml?style=flat) ![Test Status](https://img.shields.io/github/actions/workflow/status/TheGrayDot/mpqcli/tag.yml?style=flat&label=test) +![Build Status](https://img.shields.io/github/actions/workflow/status/TheGrayDot/mpqcli/tag.yml?style=flat) ![Test Status](https://img.shields.io/github/actions/workflow/status/TheGrayDot/mpqcli/tag.yml?style=flat&label=test) ![Valgrind](https://img.shields.io/github/actions/workflow/status/TheGrayDot/mpqcli/valgrind.yml?style=flat&label=valgrind) ![Release Version](https://img.shields.io/github/v/release/TheGrayDot/mpqcli?style=flat) diff --git a/test/test_about_version.py b/test/test_about_version.py new file mode 100644 index 0000000..59f29b3 --- /dev/null +++ b/test/test_about_version.py @@ -0,0 +1,84 @@ +import subprocess + +def test_version_without_arguments(binary_path): + """ + Test the version subcommand without arguments. + + This test checks: + - If the version subcommand succeeds when called without arguments. + - If the output contains version information. + """ + result = subprocess.run( + [str(binary_path), "version"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True + ) + + assert result.returncode == 0, f"mpqcli failed with error: {result.stderr}" + assert len(result.stdout.strip()) > 0, "Version output should not be empty" + # Version output should contain a dash separator (e.g., "1.0.0-abc123") + assert "-" in result.stdout, f"Version output should contain version-hash format: {result.stdout}" + + +def test_version_with_arguments(binary_path): + """ + Test the version subcommand with arguments. + + This test checks: + - If the version subcommand fails when called with arguments. + """ + result = subprocess.run( + [str(binary_path), "version", "extra_argument"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True + ) + + assert result.returncode != 0, "Version subcommand should fail when given arguments" + + +def test_about_without_arguments(binary_path): + """ + Test the about subcommand without arguments. + + This test checks: + - If the about subcommand succeeds when called without arguments. + - If the output contains expected information fields. + """ + result = subprocess.run( + [str(binary_path), "about"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True + ) + + assert result.returncode == 0, f"mpqcli failed with error: {result.stderr}" + + # Check that the output contains expected fields + output = result.stdout + assert "Name:" in output, "About output should contain 'Name:'" + assert "Version:" in output, "About output should contain 'Version:'" + assert "Author:" in output, "About output should contain 'Author:'" + assert "License:" in output, "About output should contain 'License:'" + assert "GitHub:" in output, "About output should contain 'GitHub:'" + assert "Dependencies:" in output, "About output should contain 'Dependencies:'" + assert "StormLib" in output, "About output should mention StormLib" + assert "CLI11" in output, "About output should mention CLI11" + + +def test_about_with_arguments(binary_path): + """ + Test the about subcommand with arguments. + + This test checks: + - If the about subcommand fails when called with arguments. + """ + result = subprocess.run( + [str(binary_path), "about", "extra_argument"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True + ) + + assert result.returncode != 0, "About subcommand should fail when given arguments" From 09c82e6d960f866a7e24205ea75e31583ba51e02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20Sj=C3=B6blom?= Date: Mon, 2 Mar 2026 09:49:10 +0100 Subject: [PATCH 2/3] Adding script for running Valgrind --- .github/workflows/valgrind.yml | 30 ++----- scripts/run_valgrind_tests.sh | 144 +++++++++++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 22 deletions(-) create mode 100755 scripts/run_valgrind_tests.sh diff --git a/.github/workflows/valgrind.yml b/.github/workflows/valgrind.yml index a37ee21..e1347b8 100644 --- a/.github/workflows/valgrind.yml +++ b/.github/workflows/valgrind.yml @@ -32,29 +32,16 @@ jobs: -f Dockerfile.valgrind \ . - - name: Set up Python - uses: actions/setup-python@v6 - with: - python-version: '3.11' - - - name: Install test dependencies - run: | - python -m pip install --upgrade pip - pip install -r test/requirements.txt - - name: Run Valgrind on test suite id: valgrind_tests run: | - # Create output directory - mkdir -p valgrind_logs - - # Run tests with Valgrind using Docker + # Run tests with Valgrind using the script inside Docker + # The script wraps mpqcli with Valgrind and runs pytest docker run --rm \ - -v $(pwd)/test:/mpqcli/test:ro \ - -v $(pwd)/valgrind_logs:/mpqcli/valgrind_logs:rw \ - -e MPQCLI_BINARY=/mpqcli/build/bin/mpqcli \ + -v $(pwd):/workspace:rw \ + -w /workspace \ mpqcli-valgrind \ - bash -c "cd /mpqcli && python3 -m pytest test -v --tb=short || true" + bash -c "./scripts/run_valgrind_tests.sh" echo "Test execution completed" @@ -82,9 +69,8 @@ jobs: # Create summary echo "## Valgrind Memory Leak Analysis" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY - echo "### Subcommands Tested" >> $GITHUB_STEP_SUMMARY + echo "### All Subcommands Tested via pytest suite:" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY - echo "**Tested via pytest suite:**" >> $GITHUB_STEP_SUMMARY echo "- \`create\` - Create MPQ archives" >> $GITHUB_STEP_SUMMARY echo "- \`add\` - Add files to archives" >> $GITHUB_STEP_SUMMARY echo "- \`remove\` - Remove files from archives" >> $GITHUB_STEP_SUMMARY @@ -93,8 +79,8 @@ jobs: echo "- \`read\` - Read files from archives" >> $GITHUB_STEP_SUMMARY echo "- \`verify\` - Verify archive signatures" >> $GITHUB_STEP_SUMMARY echo "- \`info\` - Display archive information" >> $GITHUB_STEP_SUMMARY - echo "- \`about\` - Display mpqcli information" >> $GITHUB_STEP_SUMMARY - echo "- \`version\` - Display mpqcli version" >> $GITHUB_STEP_SUMMARY + echo "- \`about\` - Display program information" >> $GITHUB_STEP_SUMMARY + echo "- \`version\` - Display program version" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY echo "### Results" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY diff --git a/scripts/run_valgrind_tests.sh b/scripts/run_valgrind_tests.sh new file mode 100755 index 0000000..75be1c0 --- /dev/null +++ b/scripts/run_valgrind_tests.sh @@ -0,0 +1,144 @@ +#!/bin/bash + +# Script to run pytest tests under Valgrind for comprehensive memory leak detection +# This wraps the mpqcli binary with Valgrind during test execution + +set -e + +SCRIPT_DIR="$(dirname "$(readlink -fm "$0")")" +PROJECT_DIR="$(dirname "$SCRIPT_DIR")" + +# Colors for output +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +NC='\033[0m' # No Color + +# Default values +USE_DOCKER=false +BUILD_DIR="build" +VALGRIND_LOG_DIR="$PROJECT_DIR/valgrind_logs" + +# Parse arguments +while [[ $# -gt 0 ]]; do + case $1 in + -d|--docker) + USE_DOCKER=true + shift + ;; + -h|--help) + echo "Usage: $0 [OPTIONS]" + echo "" + echo "Run pytest tests with Valgrind memory leak detection" + echo "" + echo "Options:" + echo " -d, --docker Run in Docker container" + echo " -h, --help Show this help message" + echo "" + echo "This script creates a wrapper that runs mpqcli under Valgrind" + echo "during pytest execution, providing comprehensive memory leak detection." + exit 0 + ;; + *) + echo -e "${RED}Unknown option: $1${NC}" + exit 1 + ;; + esac +done + +# Create Valgrind log directory +mkdir -p "$VALGRIND_LOG_DIR" + +if [ "$USE_DOCKER" = true ]; then + echo -e "${GREEN}Running tests with Valgrind in Docker...${NC}" + + # Build Docker image with current user's UID/GID + echo -e "${YELLOW}Building Docker image...${NC}" + docker build \ + --build-arg USER_ID="$(id -u)"\ + --build-arg GROUP_ID="$(id -g)" \ + -t mpqcli-valgrind \ + -f "$PROJECT_DIR/Dockerfile.valgrind" \ + "$PROJECT_DIR" + + # Run tests in Docker with Valgrind wrapper + echo -e "${YELLOW}Running pytest with Valgrind...${NC}" + docker run --rm -v "$PROJECT_DIR":/mpqcli mpqcli-valgrind bash -c " + # Create Valgrind wrapper script + cat > /tmp/mpqcli_wrapper.sh << 'EOF' +#!/bin/bash +valgrind --leak-check=full \ + --show-leak-kinds=all \ + --track-origins=yes \ + --log-file=/mpqcli/valgrind_logs/valgrind_%p.log \ + /mpqcli/build/bin/mpqcli \"\$@\" +EOF + chmod +x /tmp/mpqcli_wrapper.sh + + # Run tests with wrapper + cd /mpqcli + . test/venv/bin/activate + MPQCLI_BIN=/tmp/mpqcli_wrapper.sh python3 -m pytest test -s + " +else + echo -e "${GREEN}Running tests with Valgrind locally...${NC}" + + # Check if Valgrind is installed + if ! command -v valgrind &> /dev/null; then + echo -e "${RED}Error: Valgrind is not installed${NC}" + echo "Install it with: sudo apt-get install valgrind" + exit 1 + fi + + # Check if binary exists + if [ ! -f "$PROJECT_DIR/$BUILD_DIR/bin/mpqcli" ]; then + echo -e "${YELLOW}Binary not found. Building with debug symbols...${NC}" + cd "$PROJECT_DIR" + cmake -B "$BUILD_DIR" -DCMAKE_BUILD_TYPE=Debug -DBUILD_MPQCLI=ON + cmake --build "$BUILD_DIR" + fi + + # Check if Python venv exists + if [ ! -d "$PROJECT_DIR/test/venv" ]; then + echo -e "${YELLOW}Setting up Python test environment...${NC}" + python3 -m venv "$PROJECT_DIR/test/venv" + . "$PROJECT_DIR/test/venv/bin/activate" + pip3 install -r "$PROJECT_DIR/test/requirements.txt" + else + . "$PROJECT_DIR/test/venv/bin/activate" + fi + + # Create Valgrind wrapper script + WRAPPER_SCRIPT="/tmp/mpqcli_valgrind_wrapper_$$.sh" + cat > "$WRAPPER_SCRIPT" << EOF +#!/bin/bash +valgrind --leak-check=full \ + --show-leak-kinds=all \ + --track-origins=yes \ + --log-file=$VALGRIND_LOG_DIR/valgrind_\$\$.log \ + "$PROJECT_DIR/$BUILD_DIR/bin/mpqcli" "\$@" +EOF + chmod +x "$WRAPPER_SCRIPT" + + # Run tests with wrapper + echo -e "${YELLOW}Running pytest with Valgrind wrapper...${NC}" + cd "$PROJECT_DIR" + MPQCLI_BIN="$WRAPPER_SCRIPT" python3 -m pytest test -s + + # Cleanup wrapper + rm -f "$WRAPPER_SCRIPT" +fi + +echo -e "${GREEN}Tests complete!${NC}" +echo -e "${YELLOW}Valgrind logs saved to: $VALGRIND_LOG_DIR${NC}" +echo "" +echo -e "${YELLOW}Analyzing results...${NC}" + +# Check for memory leaks in logs +if grep -q "definitely lost" "$VALGRIND_LOG_DIR"/*.log 2>/dev/null; then + echo -e "${RED}Memory leaks detected! Check logs in $VALGRIND_LOG_DIR${NC}" + grep -H "definitely lost" "$VALGRIND_LOG_DIR"/*.log | head -20 + exit 1 +else + echo -e "${GREEN}No definite memory leaks detected!${NC}" +fi From 7398f8a891ffe0086a984f8273e5d6480062fb57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Johan=20Sj=C3=B6blom?= Date: Mon, 2 Mar 2026 10:26:44 +0100 Subject: [PATCH 3/3] Fixing Valgrind execution --- .github/workflows/valgrind.yml | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/.github/workflows/valgrind.yml b/.github/workflows/valgrind.yml index e1347b8..931513a 100644 --- a/.github/workflows/valgrind.yml +++ b/.github/workflows/valgrind.yml @@ -7,6 +7,7 @@ on: permissions: contents: read + pull-requests: write jobs: valgrind: @@ -33,17 +34,22 @@ jobs: . - name: Run Valgrind on test suite - id: valgrind_tests run: | - # Run tests with Valgrind using the script inside Docker - # The script wraps mpqcli with Valgrind and runs pytest + mkdir -p valgrind_logs docker run --rm \ - -v $(pwd):/workspace:rw \ - -w /workspace \ - mpqcli-valgrind \ - bash -c "./scripts/run_valgrind_tests.sh" - - echo "Test execution completed" + -v $(pwd)/test:/mpqcli/test:rw \ + -v $(pwd)/valgrind_logs:/mpqcli/valgrind_logs:rw \ + mpqcli-valgrind bash -c ' + cp /mpqcli/build/bin/mpqcli /mpqcli/build/bin/mpqcli.real + cat > /mpqcli/build/bin/mpqcli << "EOF" + #!/bin/bash + exec valgrind --leak-check=full --show-leak-kinds=all --track-origins=yes \ + --log-file=/mpqcli/valgrind_logs/valgrind_$$.log \ + /mpqcli/build/bin/mpqcli.real "$@" + EOF + chmod +x /mpqcli/build/bin/mpqcli + cd /mpqcli && python3 -m pytest test -v --tb=short + ' - name: Analyze Valgrind results id: analyze