pagure

#5508 Bump test containers to F40, bump pip version pinning to align with rpm versions, address some tech debts in dependencies
Merged by ngompa. Opened by wombelix.
wombelix/pagure tests_bump_to_f40  into  master

Download 5508.patch

Work in Progress PR (not ready to be merged) to bump test container to F40.
This includes raising version pinning in pip and align it with the package version shipped in F40 as rpm. Means it addresses a couple of tech debts as well.
I was able to bring the failed tests down to one:
FAILED tests/test_pagure_flask_dump_load_ticket.py::PagureFlaskDumpLoadTicketTests::test_dumping_reloading_ticket - Exception: Unable to find object
But even after spending hours, I didn't find a way to fix it. For me it looks like an issue how we test and that the actual pagure code is fine.
This update_ticket_from_git call (https://pagure.io/pagure/blob/master/f/tests/test_pagure_flask_dump_load_ticket.py#_194) fails because it the database return is None (https://pagure.io/pagure/blob/master/f/pagure/lib/tasks.py#_277) and not the latest ticket entry (https://pagure.io/pagure/blob/master/f/pagure/lib/query.py#_3507).
The thing is, when I throw an exception before https://pagure.io/pagure/blob/master/f/pagure/lib/tasks.py#_285 like this:

        raise pagure.exceptions.PagureException(
            "session: %s\nticketuid: %s\nobj: %s"
            % (project, ticketuid, obj)
        )

Then obj is not None and contains the expected database result. If I don't throw the exception, then the if obj is None: hits.
That sounds like some sort of weird concurrency, async whatever issue to me. In https://pagure.io/pagure/blob/master/f/tests/test_pagure_flask_dump_load_ticket.py#_172 we throw away the current db session and recreate everything again. But celery seem to continue to use the global one from before because of the task decorator we add: https://pagure.io/pagure/blob/master/f/pagure/lib/tasks_utils.py#_20
But why there is a result when I throw and exception myself but no of I let it just running, no idea yet.

The unit, or better integration tests, starting celery tasks in always eager mode (https://docs.celeryq.dev/en/stable/userguide/configuration.html#task-always-eager). Another confusing thing because we run a redis instance inside the test container and configure the communication through unix sockets. So for probably historical reasons there are a couple of weird things going on regarding testing our celery tasks in my opinion.

So currently I don't have a idea how to further troubleshoot and address the Unable to find object problem. It also becomes a time problem, I spend way too much of my non existing free time on it already :-/ I pushed the code I have so far and leave the PR open as WIP in case someone else has some ideas and can help out here.

pretty please pagure-ci rebuild

1 new commit added

  • tests: Drop mock 'pagure.lib.git._maybe_wait'

CI is still running, py39 passed already, waiting for py311 and p312 to finish.
But local tests on fedora rpm looking good already.

After digging again into the Unable to find object problem I realized that the trace included a mocked method:

tests/test_pagure_flask_dump_load_ticket.py:203: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
pagure/lib/git.py:549: in update_ticket_from_git
    issue = pagure.lib.query.new_issue(
pagure/lib/query.py:1757: in new_issue
    pagure.lib.git.update_git(issue, repo=repo)
pagure/lib/git.py:165: in update_git
    _maybe_wait(queued)
/usr/lib/python3.12/site-packages/mock/mock.py:1100: in __call__
    return _mock_self._mock_call(*args, **kwargs)
/usr/lib/python3.12/site-packages/mock/mock.py:1104: in _mock_call
    return _mock_self._execute_mock_call(*args, **kwargs)
/usr/lib/python3.12/site-packages/mock/mock.py:1167: in _execute_mock_call
    result = effect(*args, **kwargs)
tests/test_pagure_flask_dump_load_ticket.py:43: in <lambda>
    mw.side_effect = lambda result: result.get()
/usr/lib/python3.12/site-packages/celery/result.py:1026: in get
    raise self.result if isinstance(
/usr/lib/python3.12/site-packages/celery/app/trace.py:477: in trace_task
    R = retval = fun(*args, **kwargs)
pagure/lib/tasks_utils.py:36: in decorated_function
    return function(self, session, *args, **kwargs)

Checking out older logs confirms that this trace came up earlier already.

So what does _maybe_wait do? well ...

def _maybe_wait(result):
    """Function to patch if one wants to wait for finish.
    This function should only ever be overridden by a few tests that depend
    on counting and very precise timing."""
    pass

How it's used by other tests?

../tests/__init__.py:def create_maybe_waiter(method, getter):
../tests/__init__.py:        self.app.get = create_maybe_waiter(self.app.get, self.app.get)
../tests/__init__.py:        self.app.post = create_maybe_waiter(self.app.post, self.app.get)
../tests/__init__.py:    """Helper function for definitely waiting in _maybe_wait."""
../tests/test_pagure_flask_dump_load_ticket.py:    @patch("pagure.lib.git._maybe_wait")
../tests/test_pagure_lib_drop_issue.py:    @patch("pagure.lib.git._maybe_wait", tests.definitely_wait)
../tests/test_pagure_lib_drop_issue.py:    @patch("pagure.lib.git._maybe_wait", tests.definitely_wait)
../tests/test_pagure_lib_git.py:        with patch("pagure.lib.git._maybe_wait", tests.definitely_wait):

Twice in tests/test_pagure_lib_drop_issue.py, once in tests/test_pagure_lib_git.py, always with tests.definitely_wait, never in the way it's used in tests/test_pagure_flask_dump_load_ticket.py.
A test with @patch("pagure.lib.git._maybe_wait", tests.definitely_wait) ended up in the same Unable to find object exception.
After just dropping the whole patch / mock:

diff --git a/tests/test_pagure_flask_dump_load_ticket.py b/tests/test_pagure_flask_dump_load_ticket.py
index 828a668a..358adbec 100644
--- a/tests/test_pagure_flask_dump_load_ticket.py
+++ b/tests/test_pagure_flask_dump_load_ticket.py
@@ -37,10 +37,8 @@ class PagureFlaskDumpLoadTicketTests(tests.Modeltests):
     """
     @patch("pagure.lib.notify.send_email")
-    @patch("pagure.lib.git._maybe_wait")
-    def test_dumping_reloading_ticket(self, mw, send_email):
+    def test_dumping_reloading_ticket(self, send_email):
         """Test dumping a ticket into a JSON blob."""
-        mw.side_effect = lambda result: result.get()
         send_email.return_value = True
         tests.create_projects(self.session)

tests/test_pagure_flask_dump_load_ticket.py::PagureFlaskDumpLoadTicketTests::test_dumping_reloading_ticket seem to be happy after all.

I have to admit, I still not fully understanding the reasoning behind how the test was implemented.
My only explanation is, that some Celery behavior must have changed in a (major) release which then hit us after removing the version pinning and using F40.

We have the tests to rely on, and if that one now passes without mocking, I'm fine with that.

:tada: 

13:07:30    py39: OK (913.11=setup[28.66]+cmd[884.45] seconds)
13:07:30    py311: OK (906.62=setup[27.10]+cmd[879.52] seconds)
13:07:30    py312: OK (1011.03=setup[51.17]+cmd[959.85] seconds)
13:07:30    congratulations :) (2830.83 seconds)

@ngompa wanna take a look and merge if you are fine with the proposed changes?

Metadata Update from @wombelix:
- Request assigned

:thumbsup:

Pull-Request has been merged by ngompa
Metadata
ngompa
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.