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

Detect memory leaks in tests #2698

Merged

Conversation

mrgriffin
Copy link
Collaborator

@mrgriffin mrgriffin commented Feb 16, 2023

Can use KNOWN_LEAKING; to specify that a test is known to leak memory. This is not particularly useful for battle tests, but could be useful for the generic tests introduced by #2696.

The location information is available in regular game builds. Thus it is available for use in debugging leaks in-game too. In the future we should consider replacing it with NULL if NDEBUG is defined. This is not currently possible because the tests do not force NDEBUG to be undefined.

To avoid changing the size of struct MemBlocks, I have stuffed the pointer into unused bits.

Example:

--- a/test/move_effect_absorb.c
+++ b/test/move_effect_absorb.c
@@ -12,6 +12,7 @@ SINGLE_BATTLE_TEST("Absorb recovers 50% of the damage dealt")
+        void *x = Alloc(0x8);
$ make check
Absorb recovers 50% of the damage dealt: FAIL
test/move_effect_absorb.c:15: 8 bytes not freed

@mrgriffin
Copy link
Collaborator Author

mrgriffin commented Feb 16, 2023

Perhaps the semantics should be more like KNOWN_FAILING;, specifically, if you specify KNOWN_LEAKING; maybe it should fail if the test does not leak? At the moment it just suppresses the leak check, i.e. without this set your test will fail if it leaks, but that's it.

If so, how should it interact with KNOWN_FAILING;? Probably not at all? i.e. a KNOWN_FAILING; test should red-FAIL if it leaks and there's not also a KNOWN_LEAKING;?

@mrgriffin mrgriffin added the category: battle-tests Related to the automated test environment label Feb 20, 2023
@AsparagusEduardo
Copy link
Collaborator

Please solve conflicts :)

Can use KNOWN_LEAKING; to specify that a test is known to leak memory.

The location information is available in regular game builds. Thus it is
available for use in debugging leaks in-game too. In the future we
should consider replacing it with NULL if NDEBUG is defined. This is not
currently possible because the tests do not force NDEBUG to be
undefined.
@mrgriffin
Copy link
Collaborator Author

Please solve conflicts :)

Done :)

@AsparagusEduardo AsparagusEduardo merged commit 220b60f into rh-hideout:upcoming May 7, 2023
@AsparagusEduardo AsparagusEduardo mentioned this pull request May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: battle-tests Related to the automated test environment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants