Skip to content

Adding Valgrind workflow for GitHub Actions#146

Open
sjoblomj wants to merge 3 commits intoTheGrayDot:mainfrom
sjoblomj:valgrind_workflow
Open

Adding Valgrind workflow for GitHub Actions#146
sjoblomj wants to merge 3 commits intoTheGrayDot:mainfrom
sjoblomj:valgrind_workflow

Conversation

@sjoblomj
Copy link
Contributor

@sjoblomj sjoblomj commented Mar 2, 2026

This PR adds a GitHub Actions Workflow that runs Valgrind (memory leak checker) on each PR.

The PR also adds pretty useless tests for the about and version subcommands. With those tests we have a complete test suite for all subcommands. That also means Valgrind gets to check all paths.

I created this PR using AI - it all seems sensible to me, but I am not an expert at either Valgrind nor GitHub Actions Workflows.

Running locally

# Build image
docker build --build-arg USER_ID=$(id -u) --build-arg GROUP_ID=$(id -g) -t mpqcli-valgrind -f Dockerfile.valgrind .

# Run image against the list subcommand and the Patch_rt.mpq file
docker run --rm \
  -v $(pwd)/path/to/directory/with/test/mpqs:/mpqcli/test/data:ro \
  mpqcli-valgrind \
  valgrind --leak-check=full --show-leak-kinds=all \
  ./build/bin/mpqcli list /mpqcli/test/data/Patch_rt.mpq

Maybe this should be mentioned in the Readme. Up to you :)

@sjoblomj sjoblomj marked this pull request as draft March 2, 2026 09:33
@sjoblomj sjoblomj force-pushed the valgrind_workflow branch 3 times, most recently from 0f703ba to 3b2e88d Compare March 3, 2026 09:58
@sjoblomj sjoblomj force-pushed the valgrind_workflow branch from 3b2e88d to 7398f8a Compare March 3, 2026 10:14
@sjoblomj
Copy link
Contributor Author

sjoblomj commented Mar 3, 2026

The job fails because there are memory leaks. #149 fixes those, so after that is merged, this job should turn green.

@sjoblomj sjoblomj marked this pull request as ready for review March 3, 2026 10:31
@sjoblomj
Copy link
Contributor Author

sjoblomj commented Mar 3, 2026

One of the steps in the GitHub Workflow will post comments on the PR if it contains memory leaks. But it currently does not have permissions to do so. You'll need to grant those permissions to the repo. AI says:

  1. Go to your repository on GitHub: https://github.com/TheGrayDot/mpqcli
  2. Click Settings (top right)
  3. In the left sidebar, click Actions → General
  4. Scroll down to Workflow permissions
  5. Select "Read and write permissions" (instead of "Read repository contents and packages permissions")
  6. Check the box "Allow GitHub Actions to create and approve pull requests"
  7. Click Save

# 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)
Copy link
Contributor Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant