CI: Run flake8 (plus various plugins) during tests #111

nicoonoclaste · 2019-02-04T15:05:19Z

Superset of Support running tests locally, fix various warts #116 :
- Extract tests & QA tools execution to a tests.sh script.
- Reorder execution order to fail faster.
- Eliminate mutable default parameters (found by flake8-bugbear)
- Uniformize the ordeer of import lines.
- Uniformize the use of trailing commas
Automatically enforce the things that were fixed in Support running tests locally, fix various warts #116 :
- Check basic lints with flake8
- Check for PEP 8 naming conventions
  This caught an issue that should have been fixed in tests: Clarify the property-based tests #85 (a scalar named l); this likely happened because the test in question was added concurrently to tests: Clarify the property-based tests #85.
- Enable flake8-bugbear and its “opinionated” checks
  This caught a number of instances where mutable data was used in default parameters.
- Check for trailing commas where useful
- Enforce a consistent import order

pathunstrom · 2019-02-04T15:58:35Z

How do you run these tests in a Windows environment?

pathunstrom · 2019-02-04T15:59:34Z

Additionally, I am strongly against a linter as a dev requirement.

nicoonoclaste · 2019-02-04T16:05:05Z

@pathunstrom The commands moved from being in .travis.yml (which contains Bash commands) to being in a shell script; in neither case can they be readily invoked in a (native) Windows environment, so that's not a regression.

nicoonoclaste · 2019-02-04T16:14:29Z

I am strongly against a linter as a dev requirement.

Could you provide a rationale?

pathunstrom · 2019-02-04T16:17:44Z

I guess last quest, you mention mutable defaults, but I don't see any such changes in the code, am I missing them?

ppb_vector/vector2.py

tests/utils.py

nicoonoclaste · 2019-02-04T16:21:24Z

@pathunstrom I found 2 at a quick glance, can't recall offhand if there were more.
You can check commit 36976d9, which introduced that lint check (it's mentioned in the commit message).

AstraLuma · 2019-02-04T16:52:27Z

Based on discord discussion, let's break this up.

I'm willing to accept all of the source changes made by the formatting in a PR. I don't disagree with any of it.

However, we need to have further discussions on the tooling changes. I think that test.sh is good (although it needs a matching .bat), I think we agree that some level of lint is good, and that having that lint as part of the local development process as simply as possible is good.

What needs further discussion is what the exact lint ruleset should be.

nicoonoclaste · 2019-02-04T16:55:53Z

[test.sh] needs a matching .bat

Can this either happen in a follow-up PR or can one of you commit it to this branch?
There's no Windows-based CI here, and I don't have any Windows system to test with.

nicoonoclaste · 2019-02-04T16:59:22Z

What needs further discussion is what the exact lint ruleset should be.

I think we ought to iterate as false-positives come up?
Every single flake8 plugin that was added here found issues, so they are arguably useful, and I find it hard to reason about single lint rules in the abstract (though you two probably have more experience with Python linters)

nicoonoclaste · 2019-02-04T18:53:30Z

I just rebased so CI would rerun with the fix from #112

nicoonoclaste · 2019-02-04T19:33:59Z

The latest commit (40a0269) fixed the PyPy build: test.sh was accidentally attempting to run mypy under pypy3.

nicoonoclaste · 2019-02-24T11:13:21Z

What needs further discussion is what the exact lint ruleset should be.

It's been almost a month, and there's been no discussion, and no response to my comments.

nicoonoclaste · 2019-02-24T12:31:27Z

The CI issue seems to be the same issue #112 should have fixed :(

nicoonoclaste · 2019-02-24T12:43:02Z

Confirmed: we have attrs 17.3.0 installed along pytest 3.10.1 even though pytest requires attrs >= 17.4.0 since version 3.5.0.

This should mitigate pip's inability to deal with [versionned dependencies], and will hopefully stop it from continuously breaking CI. [versionned dependencies]: ppb#111 (comment)

nicoonoclaste · 2019-02-24T15:28:04Z

OK, looks like #114 fixed the CI issue (hopefully for good...)

nicoonoclaste · 2019-03-06T12:14:48Z

Ping?

nicoonoclaste · 2019-03-15T18:19:17Z

@astronouth7303 @pathunstrom I did what I suggested and split off the actual changes into #116.
This branch contains #116, plus the commits that enforce those lints in CI.

nicoonoclaste · 2019-03-18T11:22:57Z

Rebased to solve the merge conflict, and fixed the lints that broke (because I can't write correctly-styled Python unassisted).

nicoonoclaste · 2019-03-18T11:23:58Z

So, about that discussion on which lint rules are desirable, you said it's needed but all we got is me monologuing for a month-and-a-half. What will it take to make it happen?

AstraLuma · 2019-03-20T19:50:36Z

Looks like hypothesis might have found a bug? Care to investigate?

nicoonoclaste · 2019-03-23T23:07:13Z

@astronouth7303 Was the failure you mentioned one of the things that we fixed recently?

AstraLuma · 2019-03-23T23:22:53Z

I can't find the CI that's in reference to, so I have no idea.

This is useful to catch simple mistakes, unused variables or imports, ...

nicoonoclaste · 2019-03-25T09:33:12Z

Rebased again, to deal with merge conflicts.

AstraLuma · 2019-03-26T15:34:43Z

Just going for it.

bors r+

111: CI: Run flake8 (plus various plugins) during tests r=astronouth7303 a=nbraud - Superset of #116 : - [x] Extract tests & QA tools execution to a `tests.sh` script. - [x] Reorder execution order to fail faster. - [x] Eliminate mutable default parameters (found by `flake8-bugbear`) - [x] Uniformize the ordeer of `import` lines. - [x] Uniformize the use of trailing commas - Automatically enforce the things that were fixed in #116 : - [x] Check basic lints with `flake8` - [x] Check for PEP 8 [naming conventions](https://www.python.org/dev/peps/pep-0008/#naming-conventions) This caught an issue that should have been fixed in #85 (a scalar named `l`); this likely happened because the test in question was added concurrently to #85. - [x] Enable `flake8-bugbear` and its “opinionated” checks This caught a number of instances where mutable data was used in default parameters. - [x] Check for trailing commas where useful - [x] Enforce a consistent import order [PEP 8]: https://www.python.org/dev/peps/pep-0008/ [`black`]: https://github.com/ambv/black Co-authored-by: Nicolas Braud-Santoni <[email protected]>

bors · 2019-03-26T15:58:59Z

Build failed

lint

nicoonoclaste · 2019-03-26T16:19:10Z

@astronouth7303 I fixed the lint issues that got introduced into master since the last rebase.

AstraLuma · 2019-03-26T16:38:14Z

ppb_vector/vector2.py

@@ -487,7 +487,7 @@ def truncate(self: Vector, max_length: typing.SupportsFloat) -> Vector:

        Note that :py:meth:`vector.scale(max_length) <scale>` is equivalent to
        :py:meth:`vector.truncate(max_length) <truncate>` when
-        :py:meth:`max_length \< vector.length <length>`.
+        :py:meth:`max_length < vector.length <length>`.


This doesn't render right in Sphinx.

Ohno. I thought I would dodge the issue by using a unicode < sign, rather than the angled braket ;_;
Fixed with a different symbol O:3

AstraLuma · 2019-03-26T16:55:05Z

NERD.

bors r+

111: CI: Run flake8 (plus various plugins) during tests r=astronouth7303 a=nbraud - Superset of #116 : - [x] Extract tests & QA tools execution to a `tests.sh` script. - [x] Reorder execution order to fail faster. - [x] Eliminate mutable default parameters (found by `flake8-bugbear`) - [x] Uniformize the ordeer of `import` lines. - [x] Uniformize the use of trailing commas - Automatically enforce the things that were fixed in #116 : - [x] Check basic lints with `flake8` - [x] Check for PEP 8 [naming conventions](https://www.python.org/dev/peps/pep-0008/#naming-conventions) This caught an issue that should have been fixed in #85 (a scalar named `l`); this likely happened because the test in question was added concurrently to #85. - [x] Enable `flake8-bugbear` and its “opinionated” checks This caught a number of instances where mutable data was used in default parameters. - [x] Check for trailing commas where useful - [x] Enforce a consistent import order [PEP 8]: https://www.python.org/dev/peps/pep-0008/ [`black`]: https://github.com/ambv/black Co-authored-by: Nicolas Braud-Santoni <[email protected]>

bors · 2019-03-26T17:25:41Z

Build succeeded

docs
FreeBSD PYTHON:3.6
FreeBSD PYTHON:3.7
lint
Linux container:python:3.6-slim
Linux container:python:3.7-slim
macOS PYTHON:3.6.8
macOS PYTHON:3.7.2
Windows container:python:3.6-windowsservercore-1809
Windows container:python:3.7-windowsservercore-1809

nicoonoclaste · 2019-03-27T11:13:46Z

@astronouth7303 Guilty as charged <3

nicoonoclaste · 2019-03-27T11:16:13Z

On one hand, ≨ is a bit of an arcane rune, OTOH I like that it's extra explicit about v.truncate(l) and v.scale_to(l) not being always equivalent when l == v.length (Hypothesis found test vectors such that v.x != 1.0 * v.x 😭 )

nicoonoclaste mentioned this pull request Feb 4, 2019

Fixing CI #112

Merged

nicoonoclaste commented Feb 4, 2019

View reviewed changes

ppb_vector/vector2.py Outdated Show resolved Hide resolved

nicoonoclaste commented Feb 4, 2019

View reviewed changes

tests/utils.py Outdated Show resolved Hide resolved

nicoonoclaste force-pushed the qa/flake8 branch from 749144f to 4968dba Compare February 4, 2019 18:50

nicoonoclaste force-pushed the qa/flake8 branch 2 times, most recently from c6e2afa to 40a0269 Compare February 4, 2019 19:29

nicoonoclaste force-pushed the qa/flake8 branch from 40a0269 to 0e33b60 Compare February 24, 2019 11:08

nicoonoclaste mentioned this pull request Feb 24, 2019

Travis: Eagerly update dependencies #114

Merged

nicoonoclaste force-pushed the qa/flake8 branch from ef32347 to 752c5da Compare March 15, 2019 18:08

nicoonoclaste mentioned this pull request Mar 15, 2019

Support running tests locally, fix various warts #116

Merged

5 tasks

nicoonoclaste changed the title ~~CI: Support running tests locally, and run flake8~~ CI: Run flake8 (plus various plugins) during tests Mar 15, 2019

nicoonoclaste force-pushed the qa/flake8 branch from 752c5da to a2078f7 Compare March 15, 2019 18:42

nicoonoclaste force-pushed the qa/flake8 branch from dc553f6 to edfac4a Compare March 23, 2019 18:14

nicoonoclaste added 6 commits March 25, 2019 10:24

tests: Run the flake8 linter

5b830dc

This is useful to catch simple mistakes, unused variables or imports, ...

tests/flake8: Enforce naming conventions consistent with PEP 8

3db821a

tests/flake8: Enable the bugbear plugin

1c54d9b

tests/flake8: Enforce trailing commas

e965e80

tests/flake8: Enforce uniform import order

08ebe04

tests: Comply with PEP8 style

091115d

nicoonoclaste force-pushed the qa/flake8 branch from edfac4a to 091115d Compare March 25, 2019 09:29

nicoonoclaste added 2 commits March 26, 2019 17:16

Merge branch 'master' into qa/flake8

f209be6

docs/conf.py: Add missing trailing comma

72bdb04

AstraLuma requested changes Mar 26, 2019

View reviewed changes

Vector2.truncate: Eliminate invalid escape sequence in docstring

f594771

nicoonoclaste force-pushed the qa/flake8 branch from 99227a7 to f594771 Compare March 26, 2019 16:50

bors bot merged commit f594771 into ppb:master Mar 26, 2019

nicoonoclaste deleted the qa/flake8 branch March 27, 2019 11:13

Uh oh!

CI: Run flake8 (plus various plugins) during tests #111

CI: Run flake8 (plus various plugins) during tests #111

Uh oh!

Conversation

nicoonoclaste commented Feb 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pathunstrom commented Feb 4, 2019

Uh oh!

pathunstrom commented Feb 4, 2019

Uh oh!

nicoonoclaste commented Feb 4, 2019

Uh oh!

nicoonoclaste commented Feb 4, 2019

Uh oh!

pathunstrom commented Feb 4, 2019

Uh oh!

Uh oh!

Uh oh!

nicoonoclaste commented Feb 4, 2019

Uh oh!

AstraLuma commented Feb 4, 2019

Uh oh!

nicoonoclaste commented Feb 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicoonoclaste commented Feb 4, 2019

Uh oh!

nicoonoclaste commented Feb 4, 2019

Uh oh!

nicoonoclaste commented Feb 4, 2019

Uh oh!

nicoonoclaste commented Feb 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicoonoclaste commented Feb 24, 2019

Uh oh!

nicoonoclaste commented Feb 24, 2019

Uh oh!

nicoonoclaste commented Feb 24, 2019

Uh oh!

nicoonoclaste commented Mar 6, 2019

Uh oh!

nicoonoclaste commented Mar 15, 2019

Uh oh!

nicoonoclaste commented Mar 18, 2019

Uh oh!

nicoonoclaste commented Mar 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AstraLuma commented Mar 20, 2019

Uh oh!

nicoonoclaste commented Mar 23, 2019

Uh oh!

AstraLuma commented Mar 23, 2019

Uh oh!

nicoonoclaste commented Mar 25, 2019

Uh oh!

AstraLuma commented Mar 26, 2019

Uh oh!

bors bot commented Mar 26, 2019

Build failed

Uh oh!

nicoonoclaste commented Mar 26, 2019

Uh oh!

AstraLuma Mar 26, 2019

Choose a reason for hiding this comment

Uh oh!

nicoonoclaste Mar 26, 2019

Choose a reason for hiding this comment

Uh oh!

AstraLuma commented Mar 26, 2019

Uh oh!

bors bot commented Mar 26, 2019

Build succeeded

Uh oh!

nicoonoclaste commented Mar 27, 2019

Uh oh!

nicoonoclaste commented Mar 27, 2019

nicoonoclaste commented Feb 4, 2019 •

edited

Loading

nicoonoclaste commented Feb 4, 2019 •

edited

Loading

nicoonoclaste commented Feb 24, 2019 •

edited

Loading

nicoonoclaste commented Mar 18, 2019 •

edited

Loading