Testing Support/Tools for Charm Developers

[relaxdiego]

What is the plan for adding unit testing support/tooling for charm developers? From a cursory look around the code and examples (e.g. https://github.com/johnsca/charm-gitlab-k8s/blob/master/src/charm.py), it seems like the usual unittest package can be used directly alongside mocked dependency injection to get that done but wanted to ask if there's already and establishing or established way to unit test charms built on top of this framework.

[relaxdiego]

(NOTE: Sorry If this comment is meandering. I'm typing as I read more of the code)

Digging a bit deeper, I can see that ops.main instantiates the charm's class and then uses an instance of ops.framework.Framework to trigger its handlers.

Based on the above and looking at the sample charm-gitlab-k8s charm's code some more, we can probably stub/mock framework, key, and event out and then call the observers (event handlers) directly with them(?)

[relaxdiego]

Just doing a sort of "thought experiment" here where I'm writing a test for a charm that works with a fictitious cloud provider named "SomebodysComputer" ignoring a few constraints emerging from the framework to make the example test simpler:

somebodyscomputercharm/test/test_on_start_observer.py:

#!/usr/bin/env python3

import unittest
from unittest.mock import call, create_autospec, patch

from from ops.charm import StartEvent

from somebodyscomputer import SomebodysComputerCharm

class TestOnStartObserver(unittest.TestCase):

    @patch('somebodyscomputer.BlockedStatus')
    def test_happy_path(self, blocked_status_cls):
        # Set up
        event_cls = create_autospec(StartEvent)
        event = event_cls.return_value
        # Mock one or more attributes of `event` here that are relevant to this test

        # Exercise the code
        charm = SomebodysComputerCharm()
        charm.on_start(event)

        # Assert stuff
        self.assertEqual(1, blocked_status_cls.call_count)
        self.assertEqual(call('Missing db'), blocked_status_cls.call_args)

[relaxdiego]

In talking to @dshcherb yesterday, I learned that the observer just needs to inherit from framework.Object(?) so I’m going to try out a slightly different approach that’ll still result in a simple unit test.

[relaxdiego]

I tried the idea I mentioned in my last comment:

class DemoCharm(CharmBase):

    # We want to keep the charm object as "thin" as possible with zero logic.
    # All it should do is initialize the observers and bind them to events.
    # This will allow us to easily test the real logic using unittest without
    # any other dependencies.
    def __init__(self, *args):
        super().__init__(*args)
        self.framework.observe(self.on.start, DemoObserver())

class DemoObserver(framework.Object):

    def on_start(self, event):
        self.framework.model.unit.status = \
            MaintenanceStatus("It's new! It's shiny! It's quite buggy!")
        return

What I later realized though is that framework.Object and therefore DemoObserver are not just Plain Old Python Objects (POPOs) that I can simply instantiate and exercise in a unit test. Currently also experiencing this error when deploying the charm:

2020-01-30 07:19:42 ERROR juju.worker.uniter.operation runhook.go:132 hook "start" failed: exit status 1
2020-01-30 07:20:02 DEBUG start Traceback (most recent call last):
2020-01-30 07:20:02 DEBUG start   File "/var/lib/juju/agents/unit-demo-charm-0/charm/hooks/start", line 30, in <module>
2020-01-30 07:20:02 DEBUG start     main(DemoCharm)
2020-01-30 07:20:02 DEBUG start   File "lib/ops/main.py", line 172, in main
2020-01-30 07:20:02 DEBUG start     charm = charm_class(framework, None)
2020-01-30 07:20:02 DEBUG start   File "/var/lib/juju/agents/unit-demo-charm-0/charm/hooks/start", line 18, in __init__
2020-01-30 07:20:02 DEBUG start     self.framework.observe(self.on.start, DemoObserver())
2020-01-30 07:20:02 DEBUG start TypeError: __init__() missing 2 required positional arguments: 'parent' and 'key'

[jameinel]

If you look at our existing tests in 'test/test*.py' you can see that we generally have a 'create_framework' method, and then instantiate the objects with

framework = self.create_framework() obj = MyObject(framework, key)

Currently the framework does need disk to function, because the design of events is that they are persisted to disk before being emitted so that an error/crash of your code will get retried at a future point.

It shouldn't be too hard to change that around, I'll give it a bit of a poke.

[relaxdiego]

Thanks, @jameinel! @dshcherb pointed me to the same earlier, specifically this.

I've been playing around with my charm code (see here) based on those tests. However, it doesn't look like the handler is getting called at all so I'm still trying to figure out what I got wrong.

[jameinel]

I thought I ran your demo-charm and saw the status get updated, as long as I did 'git clone --recurse-submodules'. I don't know what you're running into for testing, as that branch doesn't look to have any test files in it.

On Thu, Jan 30, 2020 at 5:44 PM Mark Maglana notifications@github.com wrote:

Thanks, @jameinel https://github.com/jameinel! @dshcherb https://github.com/dshcherb pointed me to the same earlier, specifically this https://github.com/canonical/operator/blob/ae67fab579c8501864715d51f5aab20cf9cf9c3f/test/test_framework.py#L108-L147 .

I've been playing around with my charm code (see here https://github.com/relaxdiego/demo-charm/blob/improve_testability/src/charm.py) based on those tests. However, it doesn't look like the handler is getting called at all so I'm still trying to figure out what I got wrong.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/canonical/operator/issues/118?email_source=notifications&email_token=AABRQ7JFURTC3ZFFCPZSVKTRALKTFA5CNFSM4KMLLKZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKLA4WI#issuecomment-580259417, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABRQ7LOCBKD6V4SJQCC4YTRALKTFANCNFSM4KMLLKZQ .

[relaxdiego]

Sorry for the confusion @jameinel:

I thought I ran your demo-charm and saw the status get updated, as long as I did 'git clone --recurse-submodules'.

Yes, that works fine but with this code structure: https://github.com/relaxdiego/demo-charm/blob/ac81925342dd508698315baf8712bbf01c7443ea/src/charm.py

However, when I refactored the code to separate the observer (so that I can eventually test it separately) this way, it doesn't and I'm not sure what I unintentionally removed in the process.

I don't know what you're running into for testing, as that branch doesn't look to have any test files in it.

I haven't gotten to that yet. For now, I'm just focusing on the charm refactor and making sure it actually works when I deploy it. After that, I'll get to the test files.

[jameinel]

Your observer object is local in "init" so it is dead by the time the event happens. What you want is "self.observer = DemoObserver(self)" You need a reference to the observer for it to not get garbage collected. The event system intentionally uses weakrefs so it won't hold on to objects that you are no longer referencing.

On Thu, Jan 30, 2020 at 7:00 PM Mark Maglana notifications@github.com wrote:

Sorry for the confusion @jameinel https://github.com/jameinel:

I thought I ran your demo-charm and saw the status get updated, as long as I did 'git clone --recurse-submodules'.

Yes, that works fine but with this code structure: https://github.com/relaxdiego/demo-charm/blob/ac81925342dd508698315baf8712bbf01c7443ea/src/charm.py

However, when I refactored the code to separate the observer (so that I can eventually test it separately) this way https://github.com/relaxdiego/demo-charm/blob/e345a25f4d8f6f9d1cb334799d232ed07b7862e5/src/charm.py, it doesn't and I'm not sure what I unintentionally removed in the process.

I don't know what you're running into for testing, as that branch doesn't look to have any test files in it.

I haven't gotten to that yet. For now, I'm just focusing on the charm refactor and making sure it actually works when I deploy it. After that, I'll get to the test files.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/canonical/operator/issues/118?email_source=notifications&email_token=AABRQ7NX3YGT7AE2YLYQED3RALTQJA5CNFSM4KMLLKZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKLJHVI#issuecomment-580293589, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABRQ7LP4INXC5T77L3Q5V3RALTQJANCNFSM4KMLLKZQ .

[relaxdiego]

The event system intentionally uses weakrefs so it won't hold on to objects that you are no longer referencing.

Aha! Thanks again. That fixed it. Alrighty, continuing with this thing...

[relaxdiego]

Thanks again for the guidance, @jameinel and @dshcherb. Here's what I've come up with so far: https://github.com/relaxdiego/demo-charm/commit/fae93d9e81346c9bdd9df710b12cff86b01f0cba

Any feedback on my approach would be more than welcome. Afterwards, I would like to contribute parts of this to the README.

[relaxdiego]

So I keep coming back to this classic talk on testing and I get a sense that I’m not structuring my code cleanly enough due to the numerous mocks I had to prepare just to test one path of one method.

[relaxdiego]

OK so shifting to a more functional style of writing the handler seems to do the trick. Here's the new charm structure and here's the resulting test which is greatly simplified.

Granted this is a very simplistic example and it's not yet clear whether this holds up to more complex logic, I think that it's going in the right direction.

[relaxdiego]

Couldn't help myself. Had to refactor again to remove what I thought were extraneous parts.

• Newer charm structure
• Corresponding test case

[jameinel]

So you can do that, but I think you'll find that functionally, you'll want access to model (at least model.unit) to do anything interesting. You'll want to look at the relations that your in, or your config, or any other sort of information.

Functionally, the interesting thing to test is how the Unit object changes in response to events. (when I get the DB joined event, my unit reconfigures something to use that.) You can farm out some of the 'interesting' bits to helper code, passing in model.unit, or model.config, etc. So while having some sort of testing infrastructure to set up that state the way you want is necessary,

On Sat, Feb 1, 2020 at 11:52 AM Mark Maglana notifications@github.com wrote:

Couldn't help myself. Had to refactor again to remove what I thought were extraneous parts.

• Newer charm structure https://github.com/relaxdiego/demo-charm/tree/68efe14b2c7c0eb10d85c0262a9d4132ef754cf1/src
• Corresponding test case https://github.com/relaxdiego/demo-charm/blob/68efe14b2c7c0eb10d85c0262a9d4132ef754cf1/test/test_start_handler.py

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/canonical/operator/issues/118?email_source=notifications&email_token=AABRQ7K5MJVK4YJXUAOKQJ3RAUS3HA5CNFSM4KMLLKZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKQXBBI#issuecomment-581005445, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABRQ7IJEXKNDIWZOYTBP4DRAUS3HANCNFSM4KMLLKZQ .

[jameinel]

helps if I finish by thought.

While having the testing infrastructure have limited dependencies is nice, IMO you do still want to interact with something more involved than just dicts.

On Tue, Feb 4, 2020 at 3:36 PM John Meinel john@arbash-meinel.com wrote:

So you can do that, but I think you'll find that functionally, you'll want access to model (at least model.unit) to do anything interesting. You'll want to look at the relations that your in, or your config, or any other sort of information.

Functionally, the interesting thing to test is how the Unit object changes in response to events. (when I get the DB joined event, my unit reconfigures something to use that.) You can farm out some of the 'interesting' bits to helper code, passing in model.unit, or model.config, etc. So while having some sort of testing infrastructure to set up that state the way you want is necessary,

On Sat, Feb 1, 2020 at 11:52 AM Mark Maglana notifications@github.com wrote:

Couldn't help myself. Had to refactor again to remove what I thought were extraneous parts.

• Newer charm structure https://github.com/relaxdiego/demo-charm/tree/68efe14b2c7c0eb10d85c0262a9d4132ef754cf1/src
• Corresponding test case https://github.com/relaxdiego/demo-charm/blob/68efe14b2c7c0eb10d85c0262a9d4132ef754cf1/test/test_start_handler.py

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/canonical/operator/issues/118?email_source=notifications&email_token=AABRQ7K5MJVK4YJXUAOKQJ3RAUS3HA5CNFSM4KMLLKZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKQXBBI#issuecomment-581005445, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABRQ7IJEXKNDIWZOYTBP4DRAUS3HANCNFSM4KMLLKZQ .

[relaxdiego]

TBH I don't see how a clean testing infrastructure and a properly functioning charm that interacts with the relevant framework object are mutually exclusive. Here's my work so far on refactoring charm-k8s-prometheus: https://github.com/relaxdiego/charm-k8s-prometheus with some of the testing methodologies I'm mentioned here in action.

[chipaca]

As of #146 we have a testing harness to support unit testing as we understand it. While a purer testing harness is possible, this gets us almost all the way there, and should be good enough for most people at least for the time being.