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

Fix the test_valgrind.sh script running tests under valgrind #189

Merged
merged 1 commit into from
Feb 1, 2024

Conversation

ldorau
Copy link
Contributor

@ldorau ldorau commented Jan 30, 2024

The current script just checks if there is the
"ERROR SUMMARY: 0 errors from 0 contexts"
line in the output of a test.

It turned out that there can be many lines starting with
"ERROR SUMMARY:" in the output of one test - some with
errors and some without, so the current approach is wrong.
We have to check if there are no other lines starting with
"ERROR SUMMARY:" than this:
"ERROR SUMMARY: 0 errors from 0 contexts" instead.

@ldorau ldorau requested a review from a team as a code owner January 30, 2024 18:31
@igchor
Copy link
Member

igchor commented Jan 30, 2024

@ldorau, the failures for protection_flag_* testcases in provider_os_memory_config` are expected since we are triggering segfaults intentionally. I think we need some kind of blacklist/filter for tests under valgrind.

@igchor
Copy link
Member

igchor commented Jan 30, 2024

But there are still some failures in numa_modes test (after commenting out protection_flag_* tests) that are not reported after your fixes:

./test/umf_test-provider_os_memory_config (valgrind failed) - OK
./test/umf_test-scalable_pool - OK
PASSED

The reason is that the test fails under valgrind - but the failure is not a memory leak, we just fail on an assert. It looks like get_mempolicy() does not work under valgrind.

@ldorau ldorau force-pushed the Fix_valgrind_script branch from 3464460 to 9d62523 Compare January 31, 2024 15:02
@ldorau
Copy link
Contributor Author

ldorau commented Jan 31, 2024

@ldorau, the failures for protection_flag_* testcases in provider_os_memory_config` are expected since we are triggering segfaults intentionally. I think we need some kind of blacklist/filter for tests under valgrind.

I will try to add suppressions for them.

@ldorau ldorau force-pushed the Fix_valgrind_script branch from 9d62523 to 7885043 Compare January 31, 2024 19:04
@ldorau
Copy link
Contributor Author

ldorau commented Jan 31, 2024

@ldorau, the failures for protection_flag_* testcases in provider_os_memory_config` are expected since we are triggering segfaults intentionally. I think we need some kind of blacklist/filter for tests under valgrind.

I will try to add suppressions for them.

@igchor Suppressions added.

@igchor
Copy link
Member

igchor commented Jan 31, 2024

@ldorau, the failures for protection_flag_* testcases in provider_os_memory_config` are expected since we are triggering segfaults intentionally. I think we need some kind of blacklist/filter for tests under valgrind.

I will try to add suppressions for them.

@igchor Suppressions added.

And what about the failures with get_mempolicy? In the logs, I still see:
./test/umf_test-provider_os_memory_config (valgrind failed) - OK

Are we just going to ignore this? I don't really know why get_mempolicy does not work under valgrind.

@ldorau
Copy link
Contributor Author

ldorau commented Jan 31, 2024

And what about the failures with get_mempolicy? In the logs, I still see: ./test/umf_test-provider_os_memory_config (valgrind failed) - OK

Are we just going to ignore this? I don't really know why get_mempolicy does not work under valgrind.

Yes, I would ignore that. The most important is that this test passes without valgrind. Apparently valgrind changes behavior of this test. We cannot help this.

@ldorau
Copy link
Contributor Author

ldorau commented Jan 31, 2024

@ldorau ldorau force-pushed the Fix_valgrind_script branch 2 times, most recently from c15f2cc to c2093df Compare February 1, 2024 09:14
@ldorau
Copy link
Contributor Author

ldorau commented Feb 1, 2024

@ldorau ldorau force-pushed the Fix_valgrind_script branch 2 times, most recently from 10d8310 to abc4e35 Compare February 1, 2024 10:30
@DamianDuy
Copy link

LGTM

test/umf_test-provider_os_memory_config.supp Outdated Show resolved Hide resolved
.github/workflows/nightly.yml Outdated Show resolved Hide resolved
test/test_valgrind.sh Outdated Show resolved Hide resolved
test/test_valgrind.sh Outdated Show resolved Hide resolved
test/test_valgrind.sh Outdated Show resolved Hide resolved
@ldorau ldorau force-pushed the Fix_valgrind_script branch from abc4e35 to df66c25 Compare February 1, 2024 13:28
The current script just checks if there is the
"ERROR SUMMARY: 0 errors from 0 contexts"
line in the output of a test.

It turned out that there can be many lines starting with
"ERROR SUMMARY:" in the output of one test - some with
errors and some without, so the current approach is wrong.
We have to check if there are no other lines starting with
"ERROR SUMMARY:" than this:
"ERROR SUMMARY: 0 errors from 0 contexts" instead.

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
@ldorau ldorau force-pushed the Fix_valgrind_script branch from df66c25 to efd90d2 Compare February 1, 2024 13:48
@ldorau ldorau merged commit e3341a1 into oneapi-src:main Feb 1, 2024
43 checks passed
@ldorau ldorau deleted the Fix_valgrind_script branch February 1, 2024 15:21
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.

4 participants