[GitHub] cassandra-dtest pull request #40: Introduce tox and pre-commit, and fix runn...

[jolynch <gi...@git.apache.org>]

GitHub user jolynch opened a pull request:

    https://github.com/apache/cassandra-dtest/pull/40

    Introduce tox and pre-commit, and fix running tests via pytest directly

    I think we can remove some of the complexity around building/running dtests by introducing [tox](https://tox.readthedocs.io/en/latest/) support and we can remove some of the confusion about python style using [pre-commit](https://pre-commit.com/)
    
    `tox` is a pretty industry standard way to manage virtualenvs so that users don't have to think about how to properly setup virtualenvs and install dependencies and the such.
    
    Devs don't need to enter virtualenvs or worry about any of that, they can just do:
    ```
    tox -e pytest -- --cassandra-dir=<cassandra dir> -k <the test name they want to run>
    ```
    and tox takes care of creating a virtualenv with the proper version of python, installing the requirements, and executing pytest (anything after the -- is passed directly to pytest).
    
    I've also included a `dtest` target for using the run_test helper script if someone wants to, e.g.
    ```
    tox -e dtest -- --cassandra-dir=/home/josephl/pg/cassandra_trunk --dtest-tests=topology_test.py
    ```
    
    Along with adding tox I've also added hooks so that whenever a person runs either the `pytest` or `dtest` tox targets we will automatically install (and later re-use) linting pre-commit hooks that will do things like check flake8 compliance, ensure that imports are sorted and in the proper order and we can do much more (there are a [lot of hooks](https://pre-commit.com/hooks.html) we can run if we want). These checks happen incrementally on each file as it is committed so that no new style violations are introduced and over time we make progress on improving the code quality. Once the whole code base is up to snuff we could even introduce a call to `tox -e pre-commit` to force the linters to run on all files at PR time to replace the current `.travis.yaml` system.  For example once the hooks are installed via a run of `tox -e pytest` or `tox -e dtest`:
    ```bash
    $ tox -e pytest -- ... something that runs a test
    
    # Now someone tries to add an incorrect style file
    $ cat foo.py 
    import zlib
    import itertools
    
    def bad(x):
        print('too close');
    
    def form(y):
        bad(x = 2)
    
    $ git commit -m "Test"
    Trim Trailing Whitespace.................................................Passed
    Flake8...................................................................Failed
    hookid: flake8
    
    foo.py:1:1: F401 'zlib' imported but unused
    foo.py:2:1: F401 'itertools' imported but unused
    foo.py:4:1: E302 expected 2 blank lines, found 1
    foo.py:5:23: E703 statement ends with a semicolon
    foo.py:7:1: E302 expected 2 blank lines, found 1
    foo.py:8:10: E251 unexpected spaces around keyword / parameter equals
    foo.py:8:12: E251 unexpected spaces around keyword / parameter equals
    foo.py:9:1: W391 blank line at end of file
    
    Reorder python imports...................................................Failed
    hookid: reorder-python-imports
    
    Files were modified by this hook. Additional output:
    
    Reordering imports in foo.py
    ```
    Now the user has clear indications where their code does not meet expectations.
    
    They can also skip the hooks if they want:
    ```
    git commit --no-verify
    ```
    
    The second commit in this PR makes it so that running dtests directly with pytest works again. That appeared broken in master (I would get failures on test collection on a fresh checkout of master.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jolynch/cassandra-dtest use_tox

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cassandra-dtest/pull/40.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #40
    
----
commit 6e2efb025039e43c5766563a1579f5f33b62ce3d
Author: Joseph Lynch <jo...@...>
Date:   2018-11-01T04:03:28Z

    Add tox support
    
    This way users can just dive straight into running tests without
    thinking about virtual environments!
    
    Also adds pre-commit while we're here which will allow users to opt
    into good linters that apply as they commit. Any of the tox commands
    will automatically install and run the linters on every commit (this is
    very lightweight, Yelp uses it on million line repos)

commit 543f2b4f1d85b77e56b3e33597f1158567c8a4bf
Author: Joseph Lynch <jo...@...>
Date:   2018-11-01T04:19:02Z

    Fix pytest running of tests

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org

[GitHub] cassandra-dtest issue #40: Introduce tox and pre-commit, and fix running tes...

[jolynch <gi...@git.apache.org>]

Github user jolynch commented on the issue:

    https://github.com/apache/cassandra-dtest/pull/40
  
    Also I know that I need docs updates but I wanted to get feedback first before re-writing the README.md.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org