Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tox cleanups and use tox in PR tests #73

Merged
merged 13 commits into from Jul 22, 2020
Merged

Tox cleanups and use tox in PR tests #73

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
1dc8b99
Use {posargs} in tox.ini to be more flexible
dossett Jul 20, 2020
fdbaac7
Merge branch 'master' into toxcleanup
dossett Jul 20, 2020
e1f64eb
use tox in the github pr test
dossett Jul 21, 2020
c4c9755
Merge branch 'toxcleanup' of github.com:etsy/boundary-layer into toxc…
dossett Jul 21, 2020
c6e6d9d
remove some deps from github action, they are in tox
dossett Jul 21, 2020
525bec6
add comment
dossett Jul 21, 2020
3c48d33
Move coveralls out of travis
dossett Jul 21, 2020
6fbd56c
coverage path
dossett Jul 21, 2020
5936537
coverage path
dossett Jul 21, 2020
6f2f33a
Revert "coverage path"
dossett Jul 21, 2020
8bf5f12
Revert "coverage path"
dossett Jul 21, 2020
c586631
Revert "Move coveralls out of travis"
dossett Jul 21, 2020
394375c
Merge branch 'master' into toxcleanup
dossett Jul 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/python-app.yml
View file
Open in desktop
Expand Up @@ -26,13 +26,13 @@ jobs:
- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install flake8 pytest mock pytest-mock .[github]
pip install tox flake8
- name: Lint with flake8
run: |
# stop the build if there are Python syntax errors or undefined names
flake8 boundary_layer boundary_layer_default_plugin bin test --count --select=E9,F63,F7,F82 --show-source --statistics
# exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide
flake8 boundary_layer boundary_layer_default_plugin bin test --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics
- name: Test with pytest
- name: Test with tox
run: |
pytest test
tox --recreate -e py`echo ${{ matrix.python-version }} | tr -d '.'` test
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No one was more surprised this worked than me.

2 changes: 1 addition & 1 deletion .travis.yml
View file
Open in desktop
Expand Up @@ -17,7 +17,7 @@ matrix:
install:
- pip install --upgrade tox coveralls

script: tox
script: tox --recreate test
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine, but technically we are this close 👌 to not needing this file anymore! travis is still running builds but those builds only go on to power the coverage reporting service. Once we figure out how to use github actions for that, we can eliminate travis altogether.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh come to think of it, the current github actions are using the example build scripts that are provided by github, which, I believe, means that it ignores our linter configuration and potentially some other stuff? I guess we could ask ourselves whether we want to continue with tox, in which case we probably should switch the github actions to use tox, or otherwise we could eliminate tox altogether? But if we eliminate it, then it's more annoying to run the tests locally... so maybe we should make the github actions use tox?

I had no reason not to use tox with them initially, other than that I was more concerned with getting the actions to run properly at all, than i was with the specifics of how we run the tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, good point. I'd lean toward using tox for the github actions too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched the github action over to tox as part of this PR and it seems fine (amazingly). Tox is set up to use pylint and the github action is using flake8. We should drive the linting through tox as well so that it's easier for developers to lint their code without reference to the configuration in the github action.

@mchalek which tool would you prefer we standardize on? I don't really have a preference.


after_success:
- ./run_coveralls.sh
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
View file
Open in desktop
Expand Up @@ -18,7 +18,7 @@ These are the steps for submitting a proposed contribution to boundary-layer:
3. Make your changes
4. Document new functionality as appropriate
5. Add new tests if possible
6. [Recommended] Run the test suite locally with `tox`
6. [Recommended] Run the test suite locally with `tox --recreate test`
7. Push your changes to your fork
8. Send a pull request!

Expand Down
4 changes: 2 additions & 2 deletions tox.ini
View file
Open in desktop
Expand Up @@ -11,7 +11,7 @@ deps = pytest==3.8.1
extras =
github
commands =
pytest -vv test
pytest -vv {posargs}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup love it 👍


[testenv:py27]
commands =
Expand All @@ -21,7 +21,7 @@ commands =
--cov=boundary_layer_default_plugin \
--cov-report=term-missing \
--cov-report=xml \
test
{posargs}

[testenv:lint]
commands =
Expand Down