Added test harness for Bigtop. #59

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants

petevg commented Jul 6, 2016

Assists in writing unit tests for layer-apache-bigtop-base, and for
layers that inherit from it.

What do people think. Is this the right place for the test harness to live? @juju-solutions/bigdata

Added test harness for Bigtop.
Assists in writing unit tests for layer-apache-bigtop-base, and for
layers that inherit from it.
Owner

johnsca commented Jul 7, 2016

I like it, nice and clean. I'm glad that you were able to avoid patching builtins.__import__ because I think this approach will definitely be easier to debug, particularly if we have to put breakpoints inside test cases.

One question I have is why you went with mocking during setUp instead of setUpClass?

As for where it should live, I think this is reasonable for sorting out the approach, but I think our goal should be to make the common bits available to others, perhaps in a charms.layer.unit_testing module on PyPI. With that goal in mind, I'd like to get some feedback from others with a vested interest, e.g., @javacruft, @marcoceppi, @tvansteenburgh, @Stub.

For reference, the goal here is to be able to unit test layers pre-charm-build, getting around the fact that the API code for the base layers isn't available by mocking the modules provided by base layers. The caveat of this approach is that the charm code that imports the mocked modules must be imported at run-time in setUp or the test cases. The other approach to take is to not have the base layers include the APIs directly but have them be full packages on PyPI and making them test reqs, which improves the encapsulation of the layers but has the downside of disconnecting the base layer API code from the reactive code.

petevg commented Jul 7, 2016

@johnsca Thanks for the feedback. I'm not sure that this is the right place for this code to live -- I'm open to suggestions for other places to put it.

To answer you question: I mocked out during setUp, because I wanted to make sure that that last_status handler got cleared between tests, and I wasn't too worried about efficiency -- I think that it's usually better to spend a bit of extra time in setUp and tearDown, if you can guarantee clean mocks for people, especially when the mocks live in a harness, instead of being created by the person writing the tests.

petevg commented Jul 7, 2016

Come to think of it, I should maybe move the cleanup of the unit state db in to tearDown -- it's another thing that should probably be reset between tests :-)

Owner

johnsca commented Jul 7, 2016

I was thinking that mocking the imports in setUpClass might be nicer because it would allow the test's setUpClass to do the imports once instead of in every test case, but you could also do that in setUp I suppose (either way you'd have to stick them onto the class / instance to avoid the scoping issues).

For resetting the statuses, you could always add self.statuses.clear() to tearDown. Also, last_status should maybe handle the potential IndexError, though maybe there's not a useful difference between an IndexError and an AssertionError.

+import unittest
+
+
+class BigtopHarness(unittest.TestCase):
@ktsakalozos

ktsakalozos Jul 8, 2016

Member

Did this get moved to jujubigdata lib?

petevg commented Jul 19, 2016

Closing, as we are actually putting this in its own repo (jujusolutions/charms.unit)

@petevg petevg closed this Jul 19, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment