Skip to content

Mark flaky ref count tests#693

Merged
dvarrazzo merged 1 commit intopsycopg:masterfrom
nialov:692-mark-more-flaky-refcount-tests
Dec 6, 2023
Merged

Mark flaky ref count tests#693
dvarrazzo merged 1 commit intopsycopg:masterfrom
nialov:692-mark-more-flaky-refcount-tests

Conversation

@nialov
Copy link
Contributor

@nialov nialov commented Dec 4, 2023

All tests that have the 'gc' fixture are now marked with the refcount mark. The reasoning is that they demonstrate flaky behaviour and disabling them in certain CI is necessary to ensure reliable testing. See #692

Besides the suggested fixes by @dvarrazzo I moved come code from fix_proxy.py to conftest.py as I believe now that other kinds of tests are also marked in pytest_collection_modifyitems it probably does not belong in a file named fix_proxy. Let me know what you think!

This was referenced Dec 4, 2023
Closed
Closed
@nialov nialov force-pushed the 692-mark-more-flaky-refcount-tests branch from fccffb0 to d10cf01 Compare December 4, 2023 18:08
@dvarrazzo
Copy link
Member

dvarrazzo commented Dec 5, 2023

Thank you for the pr, @nialov

I don't like to have too much stuff in the conftest, so I would rather not move the proxy collection_modifyitems() there.

Could you please revert the proxy change, and instead create a new fix_gc.py module, where to move all the gc-related code: a collection_modifyitems() on its own only dealing with the gc fixture and a pytest_configure registering the refcount marker.

Thank you very much!

@nialov
Copy link
Contributor Author

nialov commented Dec 5, 2023

Sure, will do! At the latest at the end of the week.

@nialov nialov force-pushed the 692-mark-more-flaky-refcount-tests branch from d10cf01 to 57ce3b3 Compare December 6, 2023 06:52
@nialov
Copy link
Contributor Author

nialov commented Dec 6, 2023

Done and hopefully it is according to your request!

@nialov nialov mentioned this pull request Dec 6, 2023
Merged
13 tasks
@dvarrazzo
Copy link
Member

dvarrazzo commented Dec 6, 2023

Thank you, what I did is part of what I wanted, but what I meant was to move everything gc-related from conftest to the module you just created, including the fixtures that were already there: pretty much this block and all the imports it requires, and clean up the unneeded imports from conftest.py:

psycopg/tests/conftest.py

Lines 106 to 170 in 4747cdb

NO_COUNT_TYPES: Tuple[type, ...] = ()
if sys.version_info[:2] == (3, 10):
# On my laptop there are occasional creations of a single one of these objects
# with empty content, which might be some Decimal caching.
# Keeping the guard as strict as possible, to be extended if other types
# or versions are necessary.
try:
from _contextvars import Context # type: ignore
except ImportError:
pass
else:
NO_COUNT_TYPES += (Context,)
class GCFixture:
__slots__ = ()
@staticmethod
def collect() -> None:
"""
gc.collect(), but more insisting.
"""
for i in range(3):
gc.collect()
@staticmethod
def count() -> int:
"""
len(gc.get_objects()), with subtleties.
"""
if not NO_COUNT_TYPES:
return len(gc.get_objects())
# Note: not using a list comprehension because it pollutes the objects list.
rv = 0
for obj in gc.get_objects():
if isinstance(obj, NO_COUNT_TYPES):
continue
rv += 1
return rv
@pytest.fixture(name="gc")
def fixture_gc():
"""
Provides a consistent way to run garbage collection and count references.
**Note:** This will skip tests on PyPy.
"""
if sys.implementation.name == "pypy":
pytest.skip(reason="depends on refcount semantics")
return GCFixture()
@pytest.fixture
def gc_collect():
"""
Provides a consistent way to run garbage collection.
**Note:** This will *not* skip tests on PyPy.
"""
return GCFixture.collect

@nialov nialov force-pushed the 692-mark-more-flaky-refcount-tests branch from 57ce3b3 to 4e53c6b Compare December 6, 2023 13:41
@nialov
test: mark flaky ref count tests
40aacab 
All tests that have the 'gc' fixture are now marked with the refcount mark. The
reasoning is that they demonstrate flaky behaviour and disabling them in
certain CI is necessary to ensure reliable testing. See psycopg#692
@nialov nialov force-pushed the 692-mark-more-flaky-refcount-tests branch from 4e53c6b to 40aacab Compare December 6, 2023 13:44
@nialov
Copy link
Contributor Author

nialov commented Dec 6, 2023

I hope it is one step closer again :D

@dvarrazzo
Copy link
Member

dvarrazzo commented Dec 6, 2023

That looks good! Merging. I will also rebase it on the branch 3.1 and push there, so you can build the 3.1.x

@dvarrazzo dvarrazzo merged commit 52ed68a into psycopg:master Dec 6, 2023
@nialov
Copy link
Contributor Author

nialov commented Dec 6, 2023

Thanks a lot!

@dvarrazzo
Copy link
Member

dvarrazzo commented Dec 6, 2023

Done: pushed your changeset as 70ef364 on the maint-3.1 branch. If you want to build 3.1.14 you will have to patch the codebase yourself; from 3.1.15 on the changeset will be in place already.

@nialov
Copy link
Contributor Author

nialov commented Dec 6, 2023

Yep, got it. Fetching the patch from there!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nialov @dvarrazzo