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 678b3ce..42f8c12 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -25,3 +25,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..bb4be7d --- /dev/null +++ b/.github/workflows/valgrind.yml @@ -0,0 +1,146 @@ +name: Valgrind Memory Leak Check + +on: + pull_request: + branches: [ main ] + paths: + - 'src/**' + - 'extern/**' + - 'test/**' + - 'Dockerfile.valgrind' + - 'CMakeLists.txt' + - '.github/workflows/valgrind.yml' + workflow_dispatch: + +permissions: + contents: read + pull-requests: write + +concurrency: + group: valgrind-${{ github.ref }} + cancel-in-progress: true + +jobs: + valgrind: + runs-on: ubuntu-24.04 + timeout-minutes: 60 + + steps: + - name: Check out repository code + uses: actions/checkout@v6 + with: + submodules: true + + - 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: Run Valgrind on test suite + run: | + mkdir -p valgrind_logs + docker run --rm \ + -v "$(pwd)/valgrind_logs:/mpqcli/valgrind_logs:rw" \ + mpqcli-valgrind bash -c ' + mv /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 && test/venv/bin/python -m pytest test -v --tb=short + ' + + - name: Analyze Valgrind results + id: analyze + run: | + total_logs=$(find valgrind_logs -maxdepth 1 -type f -name '*.log' 2>/dev/null | wc -l) + logs_with_errors=$(grep -lE "ERROR SUMMARY: [1-9][0-9]* errors" valgrind_logs/*.log 2>/dev/null | wc -l) + logs_with_leaks=$(grep -lE "(definitely|indirectly) lost: [1-9][0-9,]* bytes" valgrind_logs/*.log 2>/dev/null | wc -l) + + echo "Total logs analyzed: $total_logs" + echo "Logs with errors: $logs_with_errors" + echo "Logs with leaks: $logs_with_leaks" + + echo "## Valgrind Memory Leak Analysis" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + + if [ "$logs_with_errors" -eq 0 ] && [ "$logs_with_leaks" -eq 0 ]; then + echo "✅ **No memory leaks detected.**" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "- Logs analyzed: $total_logs" >> $GITHUB_STEP_SUMMARY + echo "status=success" >> $GITHUB_OUTPUT + else + echo "⚠️ **Potential memory issues detected**" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "- Logs analyzed: $total_logs" >> $GITHUB_STEP_SUMMARY + echo "- Logs with errors: $logs_with_errors" >> $GITHUB_STEP_SUMMARY + echo "- Logs with leaks: $logs_with_leaks" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo "#### Logs with leaks" >> $GITHUB_STEP_SUMMARY + echo "" >> $GITHUB_STEP_SUMMARY + echo '```' >> $GITHUB_STEP_SUMMARY + grep -lE "(definitely|indirectly) lost: [1-9][0-9,]* bytes" valgrind_logs/*.log 2>/dev/null >> $GITHUB_STEP_SUMMARY || true + echo '```' >> $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@v7 + with: + name: valgrind-logs + path: | + valgrind_logs/ + retention-days: 30 + + - name: Comment on PR + if: github.event_name == 'pull_request' + uses: actions/github-script@v9 + with: + script: | + const marker = ''; + const status = '${{ steps.analyze.outputs.status }}'; + const runUrl = `${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}`; + + const { data: comments } = await github.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + }); + const existing = comments.find(c => c.body && c.body.startsWith(marker)); + + if (status === 'warning') { + const body = `${marker}\n⚠️ **Valgrind detected potential memory leaks**\n\nPlease review the [Valgrind logs](${runUrl}) for details.\n\n*Note: This check is informational and does not block merging.*`; + if (existing) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + body, + }); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body, + }); + } + } else if (existing) { + const body = `${marker}\n✅ **No Valgrind memory leaks detected.**\n\nIssues reported by an earlier run on this PR have been resolved. See the latest [Valgrind logs](${runUrl}).`; + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + body, + }); + } diff --git a/Dockerfile.valgrind b/Dockerfile.valgrind new file mode 100644 index 0000000..1689b87 --- /dev/null +++ b/Dockerfile.valgrind @@ -0,0 +1,47 @@ +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 +RUN test/venv/bin/pip 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/scripts/run_valgrind_tests.sh b/scripts/run_valgrind_tests.sh new file mode 100755 index 0000000..501931e --- /dev/null +++ b/scripts/run_valgrind_tests.sh @@ -0,0 +1,147 @@ +#!/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 "$VALGRIND_LOG_DIR":/mpqcli/valgrind_logs mpqcli-valgrind bash -c " + # Replace the binary in place with a Valgrind wrapper + mv /mpqcli/build/bin/mpqcli /mpqcli/build/bin/mpqcli.real + cat > /mpqcli/build/bin/mpqcli << '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.real \"\$@\" +EOF + chmod +x /mpqcli/build/bin/mpqcli + + # Run tests + cd /mpqcli + . test/venv/bin/activate + 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 + + # Replace the binary in place with a Valgrind wrapper + BIN="$PROJECT_DIR/$BUILD_DIR/bin/mpqcli" + REAL_BIN="$BIN.real" + mv "$BIN" "$REAL_BIN" + cat > "$BIN" << EOF +#!/bin/bash +valgrind --leak-check=full \ + --show-leak-kinds=all \ + --track-origins=yes \ + --log-file=$VALGRIND_LOG_DIR/valgrind_\$\$.log \ + "$REAL_BIN" "\$@" +EOF + chmod +x "$BIN" + + # Restore the real binary on exit + trap 'mv "$REAL_BIN" "$BIN"' EXIT + + # Run tests + echo -e "${YELLOW}Running pytest with Valgrind wrapper...${NC}" + cd "$PROJECT_DIR" + python3 -m pytest test -s +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 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"