Skip to content
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

Run Python tests under asan with leak san disabled. #3304

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dom96
Copy link
Collaborator

@dom96 dom96 commented Jan 8, 2025

These tests are currently failing on shutdown due to a memory leak. This PR re-enables them on ASAN without the leak sanitizer to get some coverage under ASAN for them.

@dom96 dom96 force-pushed the dominik/only-leak-san branch 3 times, most recently from d1d6ca5 to 8d9575b Compare January 9, 2025 18:42
@dom96 dom96 changed the title Add asan-no-leak-san config to run some Python tests under. Run Python tests under asan with leak san disabled. Jan 9, 2025
@dom96 dom96 force-pushed the dominik/only-leak-san branch from 8d9575b to b4381a1 Compare January 9, 2025 18:45
@dom96 dom96 marked this pull request as ready for review January 9, 2025 18:45
@dom96 dom96 requested review from a team as code owners January 9, 2025 18:45
@dom96 dom96 requested review from mikea, hoodmane and fhanau January 9, 2025 18:45
@@ -63,6 +63,9 @@ def _wd_test_impl(ctx):
# can't just specify some other executable with some args. OK, fine, we'll use a script that
# just execs its args.
if is_windows:
if len(ctx.attr.env):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will just make the Windows build fail right? Instead, you could use target_compatible_with to mark targets as incompatible with Windows if env is set

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, just passing env on non-Windows is probably easier and avoids just skipping the tests on Windows (assuming we won't have windows-asan in the future).

@fhanau
Copy link
Collaborator

fhanau commented Jan 9, 2025

Looks like the tests are just segfaulting instead unfortunately.

@dom96
Copy link
Collaborator Author

dom96 commented Jan 10, 2025

Huh, that's interesting. It doesn't happen when run via bazel test @workerd//src/cloudflare/internal/test/vectorize:python-vectorize-api-test_0.26.0a2@ --config=asan

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