Misleading/wrong tests?

[Xcelled]

I might be missing something here, but I'm noticing a lot of tests use this "pattern":

def test_mirror_sync_package_error_no_early_exit(mirror, requests):
    mirror.master.all_packages = mock.Mock()
    mirror.master.all_packages.return_value = {"foo": 1}

   # test code

It's mocking a nonexistent method. I removed the lines and the tests still pass. It's weird and also doesn't give me a lot of confidence in the test suite being useful, especially when considering that the code coverage reports that all of the bandersnatch classes are completely uncovered. I know they're not, but it's confusing to say the least:

[cooperlees]

Hmm - this use to show coverage when I last looked in depth (when I Python 3’d the code). I’ll try look into it. But if you work it out PRs will be accepted.

[cooperlees]

I get same results running tox (from trunk/HEAD) and then looking at the coverage HTML output. Have attached screenshot.

How I ran (from root of bandersnatch repo):

python3 -m venv /tmp/tb
/tmp/tb/bin/pip install —upgrade pip setuptools tox
/tmp/tb/bin/tox

[cooperlees]

Put up #63 To address this. Then we can look into the mocking issues ...

[yeraydiazdiaz]

I can't reproduce this issue, all_packages is defined in master.py and commenting the mock above does break the test

[cooperlees]

Thanks for trying to repro and noting this. If I get some time I'll try repro and if I can't I will close this issue. Also, if anyone else can try repro unmocking somethings etc. that would be great!

[Xcelled]

Wow I have no idea how I missed it. It's right there.

Removing the mock didn't break the tests for me, but it's possible something else was going on anyway.

[cooperlees]

I'm confident these work.