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

[BUG] test failure: test_unicode.UnicodeTest.test_raiseMemError on 32bit in Debian CI #5927

Closed
stefanor opened this issue Dec 28, 2023 · 6 comments

Comments

@stefanor
Copy link
Contributor

stefanor commented Dec 28, 2023

Describe the bug

test_unicode.UnicodeTest.test_raiseMemError is failing on i386, armhf and armel (the 32 bit ARM ports) in Debian's CI.

Logs: https://ci.debian.net/packages/c/cython/

The cause isn't obvious so we'll skip the test, but I'm filing this so that someone can investigate more deeply.

Code to reproduce the behaviour:

No response

Expected behaviour

No response

OS

Linux (Debian)

Python version

3.12.1

Cython version

3.0.6

Additional context

======================================================================
FAIL: test_raiseMemError (test_unicode.UnicodeTest.test_raiseMemError)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/run/test_unicode.pyx", line 2355, in test_unicode.UnicodeTest.test_raiseMemError (test_unicode.c:144223)
    self.assertRaises(MemoryError, alloc)
AssertionError: MemoryError not raised by lambda

======================================================================
FAIL: test_raiseMemError (test_unicode.UnicodeTest.test_raiseMemError)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/run/test_unicode.pyx", line 2355, in test_unicode.UnicodeTest.test_raiseMemError (test_unicode.cpp:144235)
    self.assertRaises(MemoryError, alloc)
AssertionError: MemoryError not raised by lambda
@stefanor stefanor changed the title [BUG] test failure: test_unicode.UnicodeTest.test_raiseMemError on 32bit ARM in Debian CI [BUG] test failure: test_unicode.UnicodeTest.test_raiseMemError on 32bit in Debian CI Dec 28, 2023
@stefanor
Copy link
Contributor Author

stefanor commented Dec 28, 2023

I'm guessing is is because we're running in a 32bit container on a 64bit kernel, so processes can allocate 4GiB

This is non-trivial to detect from userspace, I can't think of a reliable way to do so offhand.

@da-woods
Copy link
Contributor

Is Python itself passing it's test-suite?

This is a test that's ultimately copied from CPython (https://github.com/python/cpython/blob/f108468970bf4e70910862476900f924fb701399/Lib/test/test_str.py#L2451) so it'd be interesting to know if that version of the same test is passing/failing/being skipped.

@stefanor
Copy link
Contributor Author

stefanor commented Dec 28, 2023

Yep, seems to be passing: https://ci.debian.net/packages/p/python3.12/unstable/i386/40342642/#L2130 (until 3.13, it's called test_unicode)

I will say that I couldn't reproduce this one locally (an i386 container on an amd64 host), but it seems to be failing reliably on our CI infra.

@da-woods
Copy link
Contributor

Thanks for confirming it.

I'm not too worried about this test since it's mostly testing Cpython internals rather than anything we're doing so I think it's fine to ignore.

This kind of bug (where it's very very platform specific) always ends up fairly hard for us to reproduce unfortunately.

@paulgevers
Copy link

One of the maintainers of ci.debian.net here. If there is a particular command you'd like me to run in the testbed where the test failed, let me know and I'll see what I can do.

@da-woods
Copy link
Contributor

da-woods commented Feb 1, 2024

My view is we should just skip this test.

The original test I think was designed to allocate a string that's just too big and is thus guaranteed to fail. However, it's based on struct sizes that are version dependent and have shrunk a little since we copied the test. So it's possible that the test is now testing something that's probably too big but might just is possible.

It's testing a CPython implementation detail that's tested properly as part of CPython. I don't think there's much value to Cython in having this test and I don't think it's worth you spend time debugging it.

I'll make a PR to remove it shortly

da-woods added a commit to da-woods/cython that referenced this issue Feb 1, 2024
Fixes cython#5927.

I don't think this test is useful for reasons documented in the
comments, thus I propose to get rid of it rather than spend
time debugging it.
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

No branches or pull requests

3 participants