lint: add rule to verify use of harness files #2367
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Prior to this commit, the tests for the linter partially depended on project files. This coupling risks churn in the tests: if the needs of the project change (e.g. a harness file is modified), then the linter tests would need to be updated. It also made the tests more difficult to understand because the input was both larger and more complicated than necessary to exercise the relevant functionality. Execute the tests in the context of the fixture directory and introduce minimal support files that serve the same purpose as the corresponding project files.
The command-line interface to Python's "unittest" module allows users to
run tests by name, e.g.
$ test.py TestLinter.test_foo
If the tests include a period, they cannot be referenced in this way.
Rename the tests to omit the filename extension so that they may be
referenced from the command-line in this way.
Verify that every test file which references a harness file using the "includes" directive also contains at least one reference to a value defined in the harness file. To support this check, extend each harness file with a list of values which it defines.
leobalter
approved these changes
Sep 27, 2019
leobalter
reviewed
Sep 27, 2019
| @@ -0,0 +1,65 @@ | |||
| import os | |||
There was a problem hiding this comment.
I really like this check. Thanks!
Sure thing! It's a little more complicated than I first expected because we currently have inter-dependencies between harness files. That's new since the last time I contributed to the project. Making each harness file truly independent could improve the contribution experience. If we ever do that, we should simplify this check. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Verify that every test file which references a harness file using the
"includes" directive also contains at least one reference to a value
defined in the harness file.
To support this check, extend each harness file with a list of values
which it defines.
I used this check to identify the changes in gh-2366. If you folks want to
include it upstream, then that patch should be merged first.