Closed Bug 2036007 Opened 1 month ago Closed 1 month ago

test_integration_patch.py flaky since v2.14.0

Categories

(Conduit :: moz-phab, defect, P3)

Production

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: clauzilla, Assigned: sheehan)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

Steps to reproduce:

Ran the integration_patch unit tests in a clean chroot:

python -m build --wheel --no-isolation
python -m venv --system-site-packages test-env
test-env/bin/python -m installer dist/*.whl
test-env/bin/python -m pytest tests/test_integration_patch.py

For context, the quoted command lines are part of the AUR PKGBUILD, which I maintain.

Actual results:

Since v2.14.0, the test_git_patch_with_commit and test_hg_patch_with_commit tests each have a ~ 1-in-4 chance of failing randomly with a message that says:

mozphab.exceptions.NotFoundError: Repository TEST not found

Example output:

========================================================= test session starts ==========================================================
platform linux -- Python 3.14.4, pytest-9.0.3, pluggy-1.6.0
rootdir: /build/moz-phab/src/review-2.14.0
configfile: pytest.ini
collected 4 items                                                                                                                      

tests/test_integration_patch.py ..F.                                                                                             [100%]

=============================================================== FAILURES ===============================================================
______________________________________________________ test_git_patch_with_commit ______________________________________________________

m_call_conduit = <MagicMock name='call' id='139871424556944'>, m_get_diffs = <MagicMock name='get_diffs' id='139871423013152'>
m_get_revs = <MagicMock name='get_revisions' id='139871423013488'>, in_process = None
git_repo_path = PosixPath('/tmp/pytest-of-builduser/pytest-0/test_git_patch_with_commit0/git-repo')

    @mock.patch("mozphab.conduit.ConduitAPI.get_revisions")
    @mock.patch("mozphab.conduit.ConduitAPI.get_diffs")
    @mock.patch("mozphab.conduit.ConduitAPI.call")
    def test_git_patch_with_commit(
        m_call_conduit,
        m_get_diffs,
        m_get_revs,
        in_process,
        git_repo_path,
    ):
        sha = git_out("rev-parse", "HEAD").rstrip("\n")
        diff_1 = copy.deepcopy(DIFF_1)
        diff_1["fields"]["refs"][0]["identifier"] = sha
        m_get_revs.return_value = [REV_1]
        m_get_diffs.return_value = {"PHID-DIFF-1": diff_1}
        m_call_conduit.side_effect = [
            {},
            {"data": [{"phid": "PHID-REPO-1", "fields": {"vcs": "git"}}]},
            PATCH_1_DIFF,
        ]
    
>       mozphab.main(["patch", "D1", "--apply-to", "here"], is_development=True)

/build/moz-phab/src/review-2.14.0/tests/test_integration_patch.py:163: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/build/moz-phab/src/review-2.14.0/mozphab/mozphab.py:167: in main
    sys.exit(1)
/build/moz-phab/src/review-2.14.0/mozphab/mozphab.py:127: in main
    args.func(repo, args)
/build/moz-phab/src/review-2.14.0/mozphab/commands/patch.py:241: in patch
    vcs_future.result()
/usr/lib/python3.14/concurrent/futures/_base.py:443: in result
    return self.__get_result()
           ^^^^^^^^^^^^^^^^^^^
/usr/lib/python3.14/concurrent/futures/_base.py:395: in __get_result
    raise self._exception
/usr/lib/python3.14/concurrent/futures/thread.py:86: in run
    result = ctx.run(self.task)
             ^^^^^^^^^^^^^^^^^^
/usr/lib/python3.14/concurrent/futures/thread.py:73: in run
    return fn(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^
/build/moz-phab/src/review-2.14.0/mozphab/git.py:937: in check_vcs
    if self.is_cinnabar_required and not self.is_cinnabar_installed:
       ^^^^^^^^^^^^^^^^^^^^^^^^^
/build/moz-phab/src/review-2.14.0/mozphab/git.py:78: in is_cinnabar_required
    return self.vcs != self.phab_vcs
                       ^^^^^^^^^^^^^
/build/moz-phab/src/review-2.14.0/mozphab/repository.py:302: in phab_vcs
    self._phab_vcs = self.phab_repo["fields"]["vcs"]
                     ^^^^^^^^^^^^^^
/build/moz-phab/src/review-2.14.0/mozphab/repository.py:239: in phab_repo
    self._phab_repo = conduit.get_repository_by_callsign(self.call_sign)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
/build/moz-phab/src/review-2.14.0/mozphab/conduit.py:708: in get_repository_by_callsign
    return self.repository_search_single("callsigns", call_sign)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <mozphab.conduit.ConduitAPI object at 0x7f365b0ade80>, constraint = 'callsigns', value = 'TEST'

    def repository_search_single(self, constraint: str, value: str) -> dict:
        """Get the information about a single repository from Phabricator.
    
        Takes a `constraint` to be passed to `diffusion.repository.search`
        and a `value` to be passed as the value of `constraint`. `value`
        must be a single element and will be passed in a list to Conduit.
        """
        api_call_args = {"constraints": {constraint: [value]}, "limit": 1}
        data = self.call("diffusion.repository.search", api_call_args)
        if not data.get("data"):
>           raise NotFoundError("Repository %s not found" % value)
E           mozphab.exceptions.NotFoundError: Repository TEST not found

/build/moz-phab/src/review-2.14.0/mozphab/conduit.py:724: NotFoundError
---------------------------------------------------------- Captured log call -----------------------------------------------------------
ERROR    moz-phab:mozphab.py:161 Traceback (most recent call last):
  File "/build/moz-phab/src/review-2.14.0/mozphab/mozphab.py", line 127, in main
    args.func(repo, args)
    ~~~~~~~~~^^^^^^^^^^^^
  File "/build/moz-phab/src/review-2.14.0/mozphab/commands/patch.py", line 241, in patch
    vcs_future.result()
    ~~~~~~~~~~~~~~~~~^^
  File "/usr/lib/python3.14/concurrent/futures/_base.py", line 443, in result
    return self.__get_result()
           ~~~~~~~~~~~~~~~~~^^
  File "/usr/lib/python3.14/concurrent/futures/_base.py", line 395, in __get_result
    raise self._exception
  File "/usr/lib/python3.14/concurrent/futures/thread.py", line 86, in run
    result = ctx.run(self.task)
  File "/usr/lib/python3.14/concurrent/futures/thread.py", line 73, in run
    return fn(*args, **kwargs)
  File "/build/moz-phab/src/review-2.14.0/mozphab/git.py", line 937, in check_vcs
    if self.is_cinnabar_required and not self.is_cinnabar_installed:
       ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/build/moz-phab/src/review-2.14.0/mozphab/git.py", line 78, in is_cinnabar_required
    return self.vcs != self.phab_vcs
                       ^^^^^^^^^^^^^
  File "/build/moz-phab/src/review-2.14.0/mozphab/repository.py", line 302, in phab_vcs
    self._phab_vcs = self.phab_repo["fields"]["vcs"]
                     ^^^^^^^^^^^^^^
  File "/build/moz-phab/src/review-2.14.0/mozphab/repository.py", line 239, in phab_repo
    self._phab_repo = conduit.get_repository_by_callsign(self.call_sign)
                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^
  File "/build/moz-phab/src/review-2.14.0/mozphab/conduit.py", line 708, in get_repository_by_callsign
    return self.repository_search_single("callsigns", call_sign)
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^
  File "/build/moz-phab/src/review-2.14.0/mozphab/conduit.py", line 724, in repository_search_single
    raise NotFoundError("Repository %s not found" % value)
mozphab.exceptions.NotFoundError: Repository TEST not found
======================================================= short test summary info ========================================================
FAILED tests/test_integration_patch.py::test_git_patch_with_commit - mozphab.exceptions.NotFoundError: Repository TEST not found
===================================================== 1 failed, 3 passed in 4.52s ======================================================

Sometimes both fail, sometimes one of them fails, sometimes both pass.
Reverting to v2.13.0 fixes the issue for me.

Expected results:

========================================================= test session starts ==========================================================
platform linux -- Python 3.14.4, pytest-9.0.3, pluggy-1.6.0
rootdir: /build/moz-phab/src/review-2.14.0
configfile: pytest.ini
collected 4 items                                                                                                                      

tests/test_integration_patch.py ....                                                                                             [100%]

========================================================== 4 passed in 8.63s ===========================================================

Possibly regressed by 2031283?

(Feel free to unlink if I’m mistaken.)

Flags: needinfo?(sledru)
Keywords: regression
Regressed by: 2031283

(In reply to Claudia Pellegrino from comment #1)

Possibly regressed by 2031283?

(Feel free to unlink if I’m mistaken.)

I think that's right. Thank you for the report. :)

Assignee: nobody → sheehan
Severity: -- → S4
Flags: needinfo?(sledru)
Priority: -- → P3

We previously used a list-based side effect. Now that calls are
parallelized, if one call completes before another, the wrong
mocked result is returned in the test. Switch to a function
based side-effect instead, using the diff ID as the key, so the
returned value is always correct.

@Connor Sheehan [:sheehan] Thanks for the patch.

The modified test has about the same breakage rate than before. I just got 5 failures out of 16 attempts, see log.

Attachment #9575517 - Attachment description: tests: use function-based side-effect for diff fetching (Bug 2036007) r?sylvestre → tests: use function-based side-effect for threaded Conduit calls (Bug 2036007) r?sylvestre

I updated the patch with fixes for a few more tests. Can you give it another try for me?

Flags: needinfo?(clauzilla)

Yay, that did it! 16 passed out of 16 attempts.

Thanks for the fix!

Flags: needinfo?(clauzilla)

(In reply to Claudia Pellegrino from comment #7)

Yay, that did it! 16 passed out of 16 attempts.

Thanks for the fix!

Thank you for verifying! I'll close out this bug once the patch is reviewed and landed. :)

Authored by https://github.com/cgsheeh
https://github.com/mozilla-conduit/review/commit/b36ad964127d3eefbf00265c5df8922f319ee52a
[main] tests: use function-based side-effect for threaded Conduit calls (Bug 2036007) r=sylvestre

Status: NEW → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: