parentwork: In tests, mock all MusicBrainz responses

[sampsyo]

I didn't notice this when we originally merged the parentwork plugin in #3279, but its tests rely on real communication with the MusicBrainz web service, i.e., they fail if there is no network connectivity. This Travis job is an example of a spurious failure caused by network interruptions.

We need to isolate these tests by mocking the MB queries so that no network traffic is every actually sent.

@dosoe, can you please look into this?

[dosoe]

Hi! I can, but not now as I'm rather occupied these days, and when I will I will probably come back to you as I have no experience with this. If I get it well, the tests test with real Musicbrainz data and it should just request data that we would simulate to separte the network connectivity from the actual code, right?

[sampsyo]

Yeah! If you like, you can look around at other tests for MusicBrainz-related stuff that we have that also provides "mock" results from the MB API and asserts that the code processes them correctly.

[dosoe]

Hi! I do have some free time now and I'm trying to get this done. I'm wondering about mocking MB-API outputs: I'm using musicbrainzngs.get_work_by_id in the core part of the plugin (the part that would need to be tested) and I don't quite get how to tell it to get the result from a mock. I had a look at test_mb.py, test_discogs.py and test_mbsync.py and they all separate the part where they get the info from the part where they process it, while I don't see how I could do the same.

[sampsyo]

How about mocking musicbrainzngs.get_work_by_id itself? That is, you'd replace that function with one that just returns a constant work information dict, perhaps collected by just capturing the output of a "real" call to the library. Would that work?

[dosoe]

But when I run the functions and methods in parentwork, how do I tell it to use this mock function instead of the real one?

[sampsyo]

That's what the unittest.mock library does for you. There are several ways to do this mocking—check out the docs for details. But the easiest one is probably the @patch decorator, like in this test that mocks os.stat:

beets/test/test_thumbnails.py

Lines 50 to 66 in 0c033bd

	@patch('beetsplug.thumbnails.ThumbnailsPlugin._check_local_ok')
	@patch('beetsplug.thumbnails.os.stat')
	def test_add_tags(self, mock_stat, _):
	plugin = ThumbnailsPlugin()
	plugin.write_metadata = Mock()
	plugin.get_uri = Mock(side_effect={b"/path/to/cover":
	"COVER_URI"}.__getitem__)
	album = Mock(artpath=b"/path/to/cover")
	mock_stat.return_value.st_mtime = 12345
	plugin.add_tags(album, b"/path/to/thumbnail")
	metadata = {"Thumb::URI": "COVER_URI",
	"Thumb::MTime": u"12345"}
	plugin.write_metadata.assert_called_once_with(b"/path/to/thumbnail",
	metadata)
	mock_stat.assert_called_once_with(album.artpath)

[dosoe]

Ok I will try this out. Thanks! By the way, I made another PR (a draft so far) here: #3557

[sampsyo]

Here is a recent CI failure caused by these networked tests. It would be really great to get this stuff fixed. https://ci.appveyor.com/project/beetbox/beets/builds/33986042/job/gipja1xj6o6948vi

[dosoe]

I tried to look into this. I don't quite understand how mock works, though. I want to use a function that I import from the plugin that uses a network call, is there a way to mock the network call (and not the function)?
More in detail:
I test the function direct_parent_id from the plugin, so I can't mock it. This function makes network calls, can I mock these even if the function is in a different library?

[dosoe]

something like

from beetsplug import parentwork
class ParentWorkTest(unittest.TestCase, TestHelper):
    def test_direct_parent_work(self):
        mb_workid = u'2e4a3668-458d-3b2a-8be2-0b08e0d8243a'
        self.assertEqual(u'f04b42df-7251-4d86-a5ee-67cfa49580d1',
                         parentwork.direct_parent_id(mb_workid)[0])
        self.assertEqual(u'45afb3b2-18ac-4187-bc72-beb1b1c194ba',
                         parentwork.work_parent_id(mb_workid)[0])

can I mock the network call made in parentwork.direct_parent_id without mocking parentwork.direct_parent_id itself?

[sampsyo]

You can mock any function, including those from other libraries. So the trick is basically to find the "lowest-level" function that wraps a network call that you can and produce responses from that.

I don't know how to mock one call to a function without mocking all the calls, although it may be possible. However, I think what you want to mock here might be musicbrainzngs.get_work_by_id. By providing direct responses to that function, you can more or less directly avoid needing to use actual MB servers.

[dosoe]

Ok that wasn't clear to me. Yes, in this case musicbrainzngs.get_work_by_id is the one I want to mock, but I was uncertain whether if I mock it in the test it would also be mocked in the plugin when I call a function of the plugin that uses it.

[jtpavlock]

I snuck in this fix 25c6843 with #3661 that turned these tests into integration tests. Ideally, we also test our end of the code with unittests as well.

[dosoe]

I made the tests work, can you check if it is what you had in mind?