[test] Disable flaky leakcheck tests, as done in ROOT's cppyy - #85
Conversation
|
Coincidentally, =================================== FAILURES ===================================
________________ TestLEAKCHECK.test09_initializer_list_argument ________________
self = <test_leakcheck.TestLEAKCHECK object at 0x10bc79f50>
def test09_initializer_list_argument(self):
"""Leak check of passing a list as an std::initializer_list argument"""
import cppjit
cppjit.cppdef("""\
namespace LeakCheck {
int sum_il(std::initializer_list<int> l) {
int s = 0;
for (auto i : l) s += i;
return s;
}
}""")
ns = cppjit.gbl.LeakCheck
> self.check_func(ns, "sum_il", [1, 2, 3])
test_leakcheck.py:300:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
self = <test_leakcheck.TestLEAKCHECK object at 0x10bc79f50>
scope = <namespace cppjit.gbl.LeakCheck at 0x7f91a6259e10>, func = 'sum_il'
args = ([1, 2, 3],), kwds = {}
def check_func(self, scope, func, *args, **kwds):
"""Leak-check 'func', given args and kwds"""
import gc
# if tmpl_args is provided as a keyword, then this is a templated
# function that is to be found on each call python-side
tmpl_args = kwds.pop("tmpl_args", None)
# warmup function
gc.collect()
if tmpl_args is None:
getattr(scope, func)(*args, **kwds)
else:
getattr(scope, func)[tmpl_args](*args, **kwds)
# number of iterations
N = 100000
# The use of arena's, free-lists, etc. means that checking rss remains
# unreliable, unless looking for consistent jumps, so the leak check will
# be run M times and only considered failed if it "leaks" every time. In
# actual practice, the number of fails is 0, 1, or M. Note that the total
# number of gc objects tracked is always required to remain the same.
M = 3
# leak check
fail = 0
for i in range(M):
gc.collect()
pre = len(gc.get_objects())
last = self.process.memory_info().rss
if tmpl_args is None:
self.runit(N, scope, func, *args, **kwds)
else:
self.runit_template(N, scope, func, tmpl_args, *args, **kwds)
gc.collect()
assert len(gc.get_objects()) == pre
if last < self.process.memory_info().rss:
fail += 1
> assert fail < M
E assert 3 < 3
test_leakcheck.py:75: AssertionErrorI'll disable it too. |
Yes this test has failed on the nightlies a couple of times. Although the platform is always intel-mac, I agree that the testing method here is not reliable. There is one more flaky failure when comparing execution times in the numba tests but on very rare occasion. |
There was a problem hiding this comment.
I think all the added skips can be dropped in favour of a single one here
| @mark.skip(reason="disabled due to its sporadic nature, especially fragile on VMs") |
eb75129 to
ca97d14
Compare
Good to know! I prefer to keep this PR just about the leakchecks for now, but let's keep the numba tests in mind for later. |
The leak checks in test_leakcheck.py compare RSS before and after a tight call loop, which is unreliable in the presence of arenas and free-lists. The same tests are disabled in ROOT's copy of the cppyy test suite (bindings/pyroot/cppyy/cppyy/test/test_leakcheck.py), so mark them as skipped here with the same reason. This replaces the macOS-specific xfail marks on test01-test03 and test08, which the unconditional skip supersedes, and with them the now unused IS_MAC and IS_MAC_X86 imports. test09_initializer_list_argument has no counterpart upstream, as it was added along with the InitializerListConverter fix in compiler-research#54. It is skipped too: it turned out to be just as sporadic.
ca97d14 to
7e323db
Compare
|
ruff errors are unrelated, fixed in #84 |
The leak checks in test_leakcheck.py compare RSS before and after a tight call loop, which is unreliable in the presence of arenas and free-lists. The same tests are disabled in ROOT's copy of the cppyy test suite (bindings/pyroot/cppyy/cppyy/test/test_leakcheck.py), so mark them as skipped here with the same reason.
This replaces the macOS-specific xfail marks on test01-test03 and test08, which the unconditional skip supersedes, and with them the now unused IS_MAC and IS_MAC_X86 imports.
test09_initializer_list_argument has no counterpart upstream, as it was added along with the InitializerListConverter fix in #54. It is skipped too: it turned out to be just as sporadic.