1436802 - Create python unittests (or cramtests) for lint implementations

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, enhancement, P3)

Description

[Andrew Halberstadt [:ahal]]

Mozlint itself has pretty good test coverage, but the lint implementations (that live under tools/lint) don't. They do tend to be fairly simple, but there has been a few bugs that could have been caught by tests (or for which we could have added a test).

Let's stand up some basic tests so there's at least a place to expand upon.

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1436802 - [lint] Add some tests for the flake8 linter integration

This essentially tests tools/lint/python/flake8.py. Though it also adds
a basic framework for testing all the other linters as well. Getting this
added now will allow others to collaborate on adding more tests without
needing to get to 100% coverage for all linters right off the bat.

All python tests under tools/lint/test will run as part of the 'ml' task
on Linux, and the build task on Windows (OSX coverage is currently missing
for python tests).

The flake8 linter currently has a bug where the 'exclude' argument is
ignored. This is why we need to also exclude 'tools/lint/test/files' in
topsrcdir/.flake8, even though it is already listed in the
'mach_commands.py'. Other linters shouldn't need to do this, the exclusion
in 'mach_commands.py' should be good enough. See bug 1277851 for more
details.

Review commit: https://reviewboard.mozilla.org/r/230640/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/230640/

Comment 2

[Andrew Halberstadt [:ahal]]

Comment on attachment 8961824 [details]
Bug 1436802 - [lint] Add some tests for the flake8 linter integration

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/230640/diff/1-2/

Comment 3

[Andrew Halberstadt [:ahal]]

Here's pytest's documentation on fixtures in case it helps to understand this review:
https://docs.pytest.org/en/latest/fixture.html

Comment 4

[Jan Keromnes [:janx]]

Comment on attachment 8961824 [details]
Bug 1436802 - [lint] Add some tests for the flake8 linter integration

https://reviewboard.mozilla.org/r/230640/#review237580

::: tools/lint/test/test_flake8.py:3
(Diff revision 1)
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.

Nit: I believe that test files should be in the public domain. https://www.mozilla.org/en-US/MPL/license-policy/ indicates that you can use a license header like this:

```
# Any copyright is dedicated to the Public Domain.
# http://creativecommons.org/publicdomain/zero/1.0/
```

(The same probably applies to `tools/lint/test/conftest.py` as well.)

Comment 5

[Andi [:andi]]

Comment on attachment 8961824 [details]
Bug 1436802 - [lint] Add some tests for the flake8 linter integration

https://reviewboard.mozilla.org/r/230640/#review237888

Comment 6

[Andrew Halberstadt [:ahal]]

Comment on attachment 8961824 [details]
Bug 1436802 - [lint] Add some tests for the flake8 linter integration

https://reviewboard.mozilla.org/r/230640/#review237580

> Nit: I believe that test files should be in the public domain. https://www.mozilla.org/en-US/MPL/license-policy/ indicates that you can use a license header like this:
> 
> ```
> # Any copyright is dedicated to the Public Domain.
> # http://creativecommons.org/publicdomain/zero/1.0/
> ```
> 
> (The same probably applies to `tools/lint/test/conftest.py` as well.)

Ah, interesting I hadn't noticed that before! I think almost all of our test code is MPLv2 but might as well start licensing under CC. Looks like by leaving off a license it also defaults to CC, so maybe I'll just do that.

Comment 7

[Andrew Halberstadt [:ahal]]

Comment on attachment 8961824 [details]
Bug 1436802 - [lint] Add some tests for the flake8 linter integration

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/230640/diff/2-3/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ad34ac3d45a6
[lint] Add some tests for the flake8 linter integration r=andi

Comment 9

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/ad34ac3d45a6