Skip to content

Conversation

lovasoa
Copy link
Collaborator

@lovasoa lovasoa commented Oct 20, 2025

Add a CI job to run unit tests under Valgrind to detect memory safety issues.


Open in Cursor Open in Web

Copy link

cursor bot commented Oct 20, 2025

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@lovasoa
Copy link
Collaborator Author

lovasoa commented Oct 20, 2025

@cursoragent valgrind must run in the existing tests (the ones defined with a strategy matrix), not in a new different test.

And do not touch my cargo lock

Copy link

cursor bot commented Oct 20, 2025

Perfect! I've successfully updated the PR to address your feedback:

Changes Made

Integrated valgrind into the existing test matrix - Valgrind now runs within the existing test job that uses the strategy matrix (postgres, mysql, mssql, odbc)

Did not touch Cargo.lock - The commit only modifies .github/workflows/ci.yml

How it works:

  • Added a valgrind installation step before the tests
  • Set CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER environment variable to run all tests under valgrind
  • All four database configurations (postgres, mysql, mssql, odbc) will now run their tests under valgrind
  • Valgrind is configured with --error-exitcode=1 to fail CI on memory errors and --leak-check=no to focus on safety errors

The tests have been verified locally and show 0 memory errors from valgrind.


Open in Cursor Open in Web

Learn more about Cursor Agents

- Add valgrind installation step to test job
- Run all matrix tests under valgrind using CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER
- Remove separate valgrind job in favor of integration with existing tests
@cursor cursor bot force-pushed the cursor/run-tests-with-valgrind-locally-and-in-ci-6535 branch from 31ef654 to 0fa6f5c Compare October 20, 2025 21:50
@lovasoa lovasoa closed this Oct 21, 2025
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.

2 participants