Opened on 04/25/2016 at 12:52:19 PM

Closed on 05/25/2016 at 08:30:34 PM

#3985 closed change (fixed)

Unified test runner for sitescripts

Reported by: kvas Assignee: kvas
Priority: Unknown Milestone:
Module: Sitescripts Keywords:
Cc: sebastian, fhd Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29342955/

Description (last modified by kvas)

Background

We have several tests lying in test folders around sitescripts. They cover different modules and have different dependencies. It appears that they were not typically run together and there has been no attempt to automate the running of the tests of measure coverage.

If we had one centralised place to run the tests for a particular subproject of sitescripts or all the tests in a more automatic way it would give us some advantages:

  • It would be easy for anyone to run the tests without trying to figure out the implicit dependencies, test locations, etc.
  • When adding new code, or changing existing code that has no test coverage, there would be an obvious documented way to add tests for it.
  • If the work on one part of sitescripts breaks something else by side-effect, it would be less likely to go unnoticed.
  • We could generate a coverage report and know what is covered and what is not.
  • It would be easier to attach the centralised test runner to some kind of continuous integration system to make sure the test failures are immediately caught.

I think it makes sense to use Tox for this. Also since sitescripts is more like several modules than like a single module, we would like to be able to run the tests of different modules independently.

What to change

  • Add tox.ini to sitescripts with one environment per submodule (include only submodules that have tests).
  • For each environment specify the list of dependencies that are necessary to run the tests of the submodule. Configure running the existing tests of the submodule.
  • Tox should be able to run the tests without installing sitescripts in the virtualenvs that it creates (sitescripts doesn't have setup.py so this will not work). This can be achieved by adding skipsdist=true to tox section of tox.ini.
  • Set envlist in tox.ini to the list of submodules whose tests don't have any dependencies that cannot be managed by Tox (like a database server, for example). Running tox without arguments should run all the tests that are environment-independent.
  • Document in README.md how to run the existing tests and how to add new tests.

Attachments (0)

Change History (8)

comment:1 Changed on 05/03/2016 at 07:11:24 PM by kvas

  • Description modified (diff)

Run tests of different submodules separately: this is more practical for how we actually work on parts of sitescripts. Separate the Tox environments by submodule and don't use setup.py for installing sitescripts into the virtualenvs.

comment:2 Changed on 05/04/2016 at 05:11:12 PM by sebastian

Any reason to exclude tests that require a database? Sure tox cannot automatically make sure the database server is installed and configured, but at least you don't have to bother about Python dependencies, PYTHONPATH, etc. if running those tests.

Also mind adding, that we don't want to create setup.py scripts for tox but rather configure it to work without? It seems simply setting skipsdist=true does the trick.

comment:3 Changed on 05/06/2016 at 03:41:57 PM by kvas

  • Description modified (diff)

comment:4 Changed on 05/09/2016 at 04:19:31 PM by sebastian

  • Ready set

comment:5 Changed on 05/09/2016 at 05:03:41 PM by kvas

  • Owner set to kvas

comment:6 Changed on 05/23/2016 at 09:58:10 PM by kvas

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:7 Changed on 05/25/2016 at 08:29:09 PM by abpbot

A commit referencing this issue has landed:
Issue 3985 - Unified test runner for sitescripts

comment:8 Changed on 05/25/2016 at 08:30:34 PM by kvas

  • Resolution set to fixed
  • Status changed from reviewing to closed

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from kvas.
 
Note: See TracTickets for help on using tickets.