-
Notifications
You must be signed in to change notification settings - Fork 2
Adding Valgrind workflow for GitHub Actions #146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
sjoblomj
wants to merge
3
commits into
TheGrayDot:main
Choose a base branch
from
sjoblomj:valgrind_workflow
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,3 +12,5 @@ jobs: | |
| test: | ||
| uses: ./.github/workflows/test.yml | ||
| needs: build | ||
| valgrind: | ||
| uses: ./.github/workflows/valgrind.yml | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| name: Valgrind Memory Leak Check | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: [ main ] | ||
| workflow_dispatch: | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
|
|
||
| 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: Run Valgrind on test suite | ||
| run: | | ||
| mkdir -p valgrind_logs | ||
| docker run --rm \ | ||
| -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 | ||
| 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 "### All Subcommands Tested via pytest suite:" >> $GITHUB_STEP_SUMMARY | ||
| echo "" >> $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 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 | ||
|
|
||
| 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.*' | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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"] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is wanted or needed?