assert_called_once() failing in hook tests

Bug #1511474 reported by Kevin W Monroe
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
apache2 (Juju Charms Collection)
Fix Released
Undecided
Unassigned

Bug Description

I noticed a problem with the unit tests (run with 'make test'). There were 4 failures in ./hooks/tests/test_vhost_config_relation.py, all related to mock_[open|close]_port.assert_called_once():

...
    mock_open_port.assert_called_once()
AttributeError: assert_called_once
...

There is no assert_called_once method. To assert that a mock method (with no params) was indeed called once, something like this could be used:

self.assertEqual(1, mock_close_port.call_count)

That said, i don't think this is the root of the problem. The 4 failures happened in the following test cases:

test_create_vhost_template_through_config_no_protocol
test_create_vhost_missing_template
test_create_vhost_template_directly
test_create_vhost_template_through_config_with_protocol

Do we expect [open|close]_port to be called during vhost creation? The purpose of these tests seems to be checking that files are created and contain expected data. I don't see why this would involve any port calls, so I think we can get rid of all [open|close]_port references from test_vhost_config_relation.py. (warning: i've been wrong before)

Revision history for this message
Stuart Bishop (stub) wrote :

I saw this the other day with charm-helpers. An old version of the mock library has been pinned in the Python virtual env, but if the newer version is installed in the system it takes precedence. Running the tests after doing 'apt get remove python-mock python3-mock' should tell you if you are seeing the same problem, or running the tests in a fresh lxc container where it was not installed in the first place.

Revision history for this message
Kevin W Monroe (kwmonroe) wrote :

Yup stub, you're right in that older versions of mock (like v1.0.1) will not surface these errors. That doesn't make it good though :) The issue is that "assert_called_once()" doesn't exist for any version of mock, so we're not testing anything by calling it. The method I believe the test author meant to use was "assert_called_once_with" or, if args aren't needed, assert that the call_count == 1.

What's happening here is that mock 1.0.1 will happily report success for undefined methods. These would both work:

        mock_open_port.show_me_potato_salad()
        mock_open_port.assert_called_once()

In recent mocks, these would throw an AttributeError because the methods don't exist. I think it was v1.3.0 that used autospec=true by default, which is what throws AttrError when you try to mock an undefined method.

Anyway, the purpose of this bug was to point out the futility of using assert_called_once(), but more importantly to call into question the value of [open|close]_port calls during vhost creation. I'm still of the mind that we shouldn't expect these methods to get called during the vhost_create_* tests, and could safely remove those calls/asserts from test_vhost_config_relation.py.

Revision history for this message
Kevin W Monroe (kwmonroe) wrote :

Looks like jrwren fixed this way back in November -- thanks!!

http://bazaar.launchpad.net/~evarlast/charms/trusty/apache2/add-logs-interface/revision/71#hooks/tests/test_vhost_config_relation.py

The promulgated apache2 charm was just updated with this fix (along with added features) today:

https://code.launchpad.net/~evarlast/charms/trusty/apache2/add-logs-interface/+merge/278222

I just ran through 'make tests' and all looks good. Marking this as Fix Released. Thanks Jay and Cory!

Changed in apache2 (Juju Charms Collection):
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.