← Back to team overview

launchpad-reviewers team mailing list archive

Re: [Merge] lp:~jcsackett/launchpad/dont-expose-domains into lp:launchpad

 

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("foo.bar@xxxxxxxxxxxxx")
    'foo-bar'
    >>> generate_nick("foo+bar@xxxxxxxxxxx")
    '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.

-- 
https://code.launchpad.net/~jcsackett/launchpad/dont-expose-domains/+merge/62023
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/dont-expose-domains into lp:launchpad.


Follow ups

References