Merge lp:~rvb/maas-test/display-maas-version into lp:maas-test
Proposed by
Raphaël Badin
Status: | Merged |
---|---|
Approved by: | Raphaël Badin |
Approved revision: | 104 |
Merged at revision: | 109 |
Proposed branch: | lp:~rvb/maas-test/display-maas-version |
Merge into: | lp:maas-test |
Diff against target: |
132 lines (+73/-2) 4 files modified
maastest/maasfixture.py (+8/-1) maastest/tests/test_maasfixture.py (+31/-0) maastest/tests/test_utils.py (+18/-0) maastest/utils.py (+16/-1) |
To merge this branch: | bzr merge lp:~rvb/maas-test/display-maas-version |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Julian Edwards (community) | Approve | ||
Review via email: mp+200863@code.launchpad.net |
Commit message
Display the version of MAAS to be installed.
Description of the change
It would be nice to also display the source of the package but this is a start and should be good enough for now.
To post a comment you must log in.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
review: approve
Looks good, thanks for doing this very useful change. Just a couple
of small comments:
> === modified file 'maastest/ tests/test_ maasfixture. py' --- tests/test_ maasfixture. py 2013-12-18 12:04:41 +0000 +++ tests/test_ maasfixture. py 2014-01-08 16:22:42 +0000 @@ maas_logs_ maas_version( self): + kvm_fixture( ) + maas_fixture = maas_fixture( fixture) + version = tring() + mock_get_ maas_version = return_ value=version) + maas_fixture, 'get_maas_version', maas_version)
> maastest/
> maastest/
> -335,6 +335,44 @@ check_call=True), )
>
> + def test_install_
> fixture = self.make_
> self.make_
> self.getUniqueS
> mock.MagicMock(
> self.patch(
> mock_get_
Because the mock is returned from patch(), you can avoid importing
MagickMock and just do:
self. patch(maas_ fixture, 'get_maas_ version' ).return_ value = version
> + mock_logging = mock.MagicMock() + maasfixture. logging, 'info', mock_logging)
> self.patch(
And this can be:
mock_logging = self.patch( maasfixture. logging, 'info')
> + maas_fixture. install_ maas() + + self.assertEqual( + "Installing MAAS (version %s)..." % version), + maas_version. mock_calls, mock_logging. mock_calls) )
> ( + [mock.call()], + [ +
> mock.call(
> mock.call("Done installing MAAS."), + ]), +
> (mock_get_
I think this is definitely a case where having more than one assertion
is better than compounding them into a single call. This is extremely
hard to read and digest. And ease of reading trumps everything else!
> + + def test_get_ maas_version_ returns_ maas_version( self): + kvm_fixture( ) + maas_fixture = maas_fixture( fixture) + version = tring() + policy = "Candidate: %s\n" % get_maas_ version( ) + + run_command. assert_ has_call( + mock.call( + kvm_fixture. _make_apt_ get_install_ command( + pack-en' ]), + check_call=True), +
> fixture = self.make_
> self.make_
> self.getUniqueS
> version + self.patch(fixture, 'run_command',
> mock.MagicMock( + return_value=(0, policy, b'stderr')))
> + + extracted_version = maas_fixture.
> fixture.
> maas_fixture.
> ['language-
> )
You could make this easier to read by assigning a variable to the
result of mock.call() and using that in the assert_has_call.
Cheers! www.enigmail. net/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://
iEYEARECAAYFAlL N8WUACgkQWhGlTF 8G/HcjiQCffd/ lSuhJw3fupbpVCe fJLGuQ xqqARnVYeifwSGl +/
eSEAn1YZRK6NYjQ
=CJXi
-----END PGP SIGNATURE-----