launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Review: Approve

Yes, we only need to test for the testrunner configuration.  I'd still like to see a cleanup as suggested in my inline comment, and you need to set a commit message on this MP, but after you've done those let me know and I can land this for you.

Diff comments:

> 
> === modified file 'lib/lp/registry/browser/tests/test_person.py'
> --- lib/lp/registry/browser/tests/test_person.py	2015-07-21 09:04:01 +0000
> +++ lib/lp/registry/browser/tests/test_person.py	2015-08-05 03:11:08 +0000
> @@ -291,6 +291,14 @@
>          view = create_initialized_view(person, '+index')
>          self.assertThat(view.page_description, Equals(person_description))
>  
> +    def test_person_view_change_password(self):
> +        person = self.factory.makePerson()
> +        view = create_initialized_view(person, '+index', principal=person)
> +        with person_logged_in(person):
> +            markup = self.get_markup(view, person)
> +        self.assertTrue('href="http://testopenid.dev/";' in markup)
> +        self.assertTrue('Change password' in markup)

In general, prefer self.assertIn('foo', markup) over self.assertTrue('foo' in markup); the former gives clearer errors in failure cases.  I also think it would be worth using a proper soupmatcher here so that you're actually testing that both these pieces belong to the same tag.

> +
>      def test_assigned_blueprints(self):
>          person = self.factory.makePerson()
>          public_spec = self.factory.makeSpecification(


-- 
https://code.launchpad.net/~bryanquigley/launchpad/devel/+merge/266247
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.