[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#754143: Fix the tests to not pollute the data directory



On Wed, 09 Jul 2014, Christophe Siraut wrote:
> > I'm not sure what's the best approach but I see two clean solutions:
> > 
> > - something global implemented as a derivative class of TestCase that
> >   does the required directory creation and settings change in self.setUp
> >   and drops the directory with a function recorded with self.addCleanup
> 
> See attached patch. Feel free to adapt, I am unsure where to put the
> subclass bits.

Thanks, it's a good base but here are more things that I'd like to see:

1/ we move this to distro_tracker.test and and we provide replacements for
   all of SimpleTestCase, TestCase and LiveServerTestCase
   (we should use a mixin class to factorize the code between
   all those)
   ALTERNATIVE: we don't do all the classe but just the mixin
   and we modify only the test cases that need it to insert
   the mixin there...

2/ we override all the settings which are derived from
   DISTRO_TRACKER_DATA_PATH (even directories which are only
   used to read data should point to empty directories and not
   to directories with possible production data)
   
3/ ideally we should also restore the settings on tearDown so that we
   don't interfer with tests that are in standard unittest.TestCase...

4/ we add unit tests for the new classes (except maybe for
   LiveServerTestCase which might be a bit heavy...) to make
   sure that the settings point to empty directories which
   are outside of DISTRO_TRACKER_BASE_PATH

5/ some docstring documentation of the new classes would be welcome

Once this is done we obviously have some cleanups to do:

* change all our tests to use those classes
* get rid of core.tests.common.temporary_media_dir() since we handle that
  at the class level now
* merge rest of distro_tracker.core.tests.common in distro_tracker.test

Cheers,
-- 
Raphaël Hertzog ◈ Debian Developer

Discover the Debian Administrator's Handbook:
→ http://debian-handbook.info/get/


Reply to: