Skip to content

Conversation

@f-dangel
Copy link
Owner

@f-dangel f-dangel commented Nov 27, 2025

For unknown reason, running the memory leak test in the same pytest session as the other tests breaks the memory leak detection. The solution is to run this test in a separate pytest session.

Also updates the CI to always run the full test suite so we don't accidentally merge another failure of the full tests into master.

Copilot AI review requested due to automatic review settings November 27, 2025 22:40
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses a memory leak test detection issue by running the memory leak test in a separate pytest session. The PR modifies test execution to isolate test_memory_leak from other tests, adds debugging output to track memory usage, and simplifies the GitHub Actions workflow configuration.

Key changes:

  • Splits test execution into two separate pytest runs: one for test_memory_leak and one for all other tests
  • Adds memory usage debugging output to the memory leak detection function
  • Simplifies GitHub Actions workflow trigger configuration

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
test/test___init__.py Added debug print statement and changed loop variable name from _ to s for memory tracking
makefile Split all test targets to run memory leak tests separately using -k filters
.github/workflows/test.yaml Simplified workflow triggers and removed conditional test execution logic

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@f-dangel f-dangel merged commit 366bcd6 into master Nov 28, 2025
19 checks passed
@f-dangel f-dangel deleted the fix-tests branch November 28, 2025 03:18
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