[Mailman-Developers] [CLI Project] Introduced Unit tests to CLI

Abhilash Raj raj.abhilash1 at gmail.com
Mon Jun 2 08:23:00 CEST 2014


Hi Rajeev,

Rajeev S <rajeevs1992 at gmail.com> wrote:

> Hi,
> 
> As per Abhilash's comment to follow the TDD, I have covered
> the test cases for the `domain` scope.

Thanks for considering my suggestion. I think you don't need to create
a sub-module for each scope inside `tests` module. It should be simpler
like `/tests/test_domain.py` and all the tests related to domains
should reside inside that. You can add multiple test-cases for Domain
like add, delete or whatever inside that single test_domain.py.

> I know that TDD requires to write tests before implementation.
> However, I believe that it's not a good idea to postpone test cases for the
> existing units further.
> 
> Also, I would like to thank Abhilash for the comment, before its too late
> to begin writing the tests.

Glad I could help :)

> I have pushed the r56 that includes the test scripts.
> 
> I have also done some significant refactoring on the code this weekend, to
> facilitate easy usage with the unittest package and to make the code 
> more cleaner.
> The refactoring mainly includes imposing the of DRY principle -- moving 
> repetitive code
> to the lib/* directory.
> 
> As an end note, this is a hefty revision. :)
> 
> http://bazaar.launchpad.net/~rajeevs1992/mailman.client/mailmancli/revision/56

I see that you parsing the configuration to return a
`mailmanclient.Client()` object twice, once in cmdparser.py and second
time in`/lib/mailman_utils.py`. I don't understand why? Can
cmdparser.py not use the `MailmanUtils.connect()` method?

Here :/src/mailmanclient/cli/core/domains.py#L32 docstrings should be
immediately after the `def` line, there should not be a empty-line in
between them. 

Here: /src/mailmanclient/cli/core/users.py#L41 you can use upto 80chars
in docstrings too before introducing a line break.

Also if you are following the convention of specifying the parameter of
a function in docstring, add a `:type param: foo` for each parameter
too. It helps sphinx in generating better documentation. 
See this: 
  https://pythonhosted.org/an_example_pypi_project/sphinx.html

Everything else looks nice.

-- 
thanks,
Abhilash Raj


More information about the Mailman-Developers mailing list