Code review comment for lp:~jcsackett/launchpad/dont-expose-domains

Revision history for this message
Benji York (benji) wrote :

The conversion from doctest to unittest looks like it could use some
more fleshing out.

Two of the scenarios from the doctest that look important aren't
represented in the unittests:

    >>> generate_nick("<email address hidden>")
    'foo-bar'
    >>> generate_nick("<email address hidden>")
    'foo+bar'

The second half of the doctest (starting with "It should be nearly
impossible to..." and going through the testing of
NicknameGenerationError) looks important too and those tests aren't
represented in the unittests.

Here are some smaller things that occurred to me during the review:

The regex in lib/lp/registry/model/person.py doesn't need the second set
of capturing parens now that we're not using the domain name.

These two lines from the end of test_can_create_noncolliding_nicknames
appear to be copypasta (and therefore can be removed):

    self._useNicknames(['bar-c'])
    self.assertEqual('bar-c', nick)

It would be nice if test_enforces_minimum_length asserts that "i" isn't
registered at test start to ensure that we're really seeing
nick-too-short behavior and not nick-already-taken behavior.

« Back to merge proposal