Previously: #4068, #4082, #3912

I think this might be all of it

Related commit: https://pagure.io/fork/tkopecek/koji/c/cb87ace577d552b4da4f477584c908d0c75bb611?branch=pr4132a

I'm not sure about adding tearDown to CliTestCase. It's an abstract base class with no setUp. Inheriting classes do not currently use super in their own tearDown methods.

If we're still worried we've missed cases, then it would probably be better to add assertions that mock._patch._active_patches is empty in some key setUp handlers.

Was there a problem caused by the absence of the mocks you've added in the above branch?

Hmm, CliTestCase is leftover from testing and shouldn't be there. Rest of them has shown up when I've tried to run those tests in parallel. It has shown that many of them conflicts on non/mocked context. There are stil some CLI cases which are failing this way, but I hadn't time to find their root causes. + maybe it could be part of #4020 instead.

Thanks for pointing out #4020. This actually uncovers a number of problems in the unit tests.

Many of the failures in parallel tests boil down to missing cleanup in other tests. The cases you have fixed above are tests that only manage to succeed because previous tests did not clean up and left mock data in place. They fail in some parallel runs and also if run alone.

That missed cleanup includes:

• tests that change the timezone, but fail to restore it
• tests that alter koji.context.context in place rather than with a mock

There's also one set of tests that fail because of sys.argv variation in parallel runs.

With these changes, I can get a successful run of pytest -n 4

For local testing, I added a global fixure to check for various leaks in unit tests. This is what I had:

[mikem@localhost koji]$ cat tests/conftest.py 
# XXX
import pytest
@pytest.fixture(autouse=True)
def run_around_tests():
    # Code that will run before your test
    import time
    assert time.tzname == ('EST', 'EDT')
    import mock
    assert not mock.mock._patch._active_patches
    from koji.context import context
    assert not context._tdict
    # A test function will be run at this point
    yield
    # Code that will run after your test
    ...

So, no more leaks of these three types (timezone, mocks, context) remain.

Nice, works for me also. Do we also want to update tox.ini to use pytest -n auto now?

I updated the test3 target. I'm hesitant to adjust the older py2 setup

Hmm, multiple problems even here.
1) Using external pytest will not use coverage modules installed in venv. If I don't have them in system env, pytest will not start
2) If I use original call with coverage it will pass the tests but don't capture the coverage logs. Probably the reason you've changed this.
3) if I try to use the venv version via python -m pytest -n auto --cov --cov-config .coveragerc3 --cov-report=html I'm getting following errors. Weird pytest version?

FAILED tests/test_hub/test_repo_requests.py::TestRequestRepo::test_request_last - TypeError: tuple indices must be integers or slices, not str
FAILED tests/test_hub/test_repo_requests.py::TestQueueTask::test_queue_task_event - ValueError: too many values to unpack (expected 2)
FAILED tests/test_hub/test_repo_requests.py::TestRequestRepo::test_request_priority - TypeError: tuple indices must be integers or slices, not str
FAILED tests/test_hub/test_repo_requests.py::TestUpdateEndEvents::test_event_cache - TypeError: tuple indices must be integers or slices, not str
FAILED tests/test_hub/test_repo_requests.py::TestDefaultMinEvent::test_lag_override - TypeError: tuple indices must be integers or slices, not str
FAILED tests/test_hub/test_repo_requests.py::TestUpdateEndEvents::test_update - TypeError: tuple indices must be integers or slices, not str
FAILED tests/test_hub/test_repo_requests.py::TestDefaultMinEvent::test_lag_override_invalid - TypeError: tuple indices must be integers or slices, not str
FAILED tests/test_hub/test_repo_requests.py::TestAutoRequests::test_auto_lag_window - TypeError: tuple indices must be integers or slices, not str
FAILED tests/test_hub/test_repo_requests.py::TestDefaultMinEvent::test_simple_lag - TypeError: tuple indices must be integers or slices, not str
FAILED tests/test_hub/test_repo_requests.py::TestDefaultMinEvent::test_tag_older - TypeError: tuple indices must be integers or slices, not str
FAILED tests/test_hub/test_repo_requests.py::TestGetRepo::test_get_repo_at_event - TypeError: 'in <string>' requires string as left operand, not list
FAILED tests/test_hub/test_repo_requests.py::TestDefaultMinEvent::test_window - TypeError: tuple indices must be integers or slices, not str
FAILED tests/test_hub/test_repo_requests.py::TestGetRepo::test_get_repo_min_event - TypeError: 'in <string>' requires string as left operand, not list
FAILED tests/test_kojira/test_repo_manager.py::RepoManagerTest::test_update_repos - AttributeError: 'function' object has no attribute 'assert_called_once'. Di...
FAILED tests/test_lib/test_utils.py::TestRmtree::test_rmtree_child - TypeError: tuple indices must be integers or slices, not str

Weird pytest version?

I get mock 2.0.0 in the env, 4.0.3 on my laptop. We do in fact have mock<=2.0.0 in test-requirements. It's been there as long as we've had the file, with no reason given in the commit message (nor in #2049). The error seems to be api variation for call_args.

It's weird that this only fails now. We must not have been actually using the venv mock before.

This seems to be working well here. The coverage was showing devtools/check-api and /tmp/kojikamid.*, so I filtered them.

/tmp/kojikamid.4gdhdo4k                 851    750    12%
/tmp/kojikamid.jhi_bexq                 851    750    12%
/tmp/kojikamid.jo2clt0w                 851    750    12%
/tmp/kojikamid.spz_8fzh                 851    690    19%
devtools/check-api                      272     54    80%

Unfortunately, kojikamid is a bit of a strange case. We should probably fix the coverage calculation for it, but reporting it 4x in /tmp is not helpful. In the meantime, this is no worse than the current master branch.

Cool, works now - ready for merge?

Cool, works now - ready for merge?

sure!

Commit bf6f1f4a fixes this pull-request