Common play skill tests #2459
Common play skill tests #2459
Conversation
d90004e to
7af9606
Compare
| bus = mock.Mock(name='bus') | ||
| audioservice = AudioService(bus) | ||
|
|
||
| # Test proper uri |
There was a problem hiding this comment.
should each of these be separate tests? IMO each execution of code and result check is a separate test.
There was a problem hiding this comment.
No, the it's just variations of input so I don't think it should be separate tests. If each check required a bunch of extra setups separating the tests would be called for but not when it's as simple as this.
|
|
||
| class TestAudioService(unittest.TestCase): | ||
| def test_lifecycle(self): | ||
| bus = mock.Mock(name='bus') |
There was a problem hiding this comment.
add a setup method and put the bus and audioservice init in there. make them instance attributes that can be used in each test. reduce, reuse, recycle! :)
There was a problem hiding this comment.
I personally want the tests to be as easy to follow as possible without jumping around to see exactly how the setup was done. If this requires some duplication of setup code that's fine by me.
There was a problem hiding this comment.
Especially since one of the tests include the init I don't want it to be in the setup
7af9606 to
a417efb
Compare
|
I refactored out the setup code in the test_common_play_skill since after looking at it again it was slightly complex and it made sense to move it out. I also split up the test of match play query and moved it to a separate testcase class to make sure the test was grouped logically. The test_audioservice setup was so trivial so I left it as is, moving it to class members just makes it more fragmented and difficult to read. |
a417efb to
1524576
Compare
|
Did some more refactoring of the tests, the multi tests have been split out into separate classes to make sure it's shown that they're related. Some of the code duplication has been removed while some was kept to keep the logic of the test clear. |
Description
This adds unittests for the common play framework along with the semi-replated audioservice interface. Resolves #2332
There are a couple of minor fixes and improvements
How to test
Ensure that the travis tests passes.
Contributor license agreement signed?
CLA [ Yes]