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.

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

Apply black on tests #1623

Closed
wants to merge 3 commits into from

Conversation

MVrachev
Copy link
Collaborator

Related to: #1582

Description of the changes being introduced by the pull request:

Currently, we are using 4 tools: black, pylint, isort and mypy.
We want to run all 4 of them on our tests for the new codebase to make
it more readable and maintainable, but we don't want to run it on the
old test files as they are following the old style guidelines.

In order to achieve that we want to use an exclusion list instead of
an inclusion list as the new code base will be developed in the future
and new test files will appear. On another hand, we don't expect any
more additional test files against the old code, so this list is static.

I decided to hardcode the names we want to exclude because that way we
won't have to remove the Git history, as opposed to renaming or moving
the old test files.
Even though the list is big around 30 files, I think this solution is
fine as this list will only contain files testing the old code meaning
the list will have static content.

I will apply each of the linters in a separate pr.

Currently, we are using 4 tools: black, pylint, isort and mypy.
We want to run all 4 of them on our tests for the new codebase to make
it more readable and maintainable, but we don't want to run it on the
old test files as they are following the old style guidelines.

In order to achieve that we want to use an exclusion list instead of
an inclusion list as the new code base will be developed in the future
and new test files will appear. On another hand, we don't expect any
more additional test files against the old code, so this list is static.

I decided to hardcode the names we want to exclude because that way we
won't have to remove the Git history, as opposed to renaming or moving
the old test files.
Even though the list is big around 30 files, I think this solution is
fine as this list will only contain files testing the old code meaning
the list will have static content.

I will apply each of the linters in a separate pr.

Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
All of the changes included are a result of applying black
on our tests on the new code.

Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
@MVrachev
Copy link
Collaborator Author

After researching the only solution I found to exclude specific tests was to introduce a new config file pyproject.toml and use a long multiline regular expression. The """ are needed to allow splitting the regular expression into separate lines.
I know it's not beautiful, but at least it does what we want without changing the Git history.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1354989188

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.325%

Totals Coverage Status
Change from base Build 1337495560: 0.0%
Covered Lines: 3955
Relevant Lines: 4042

💛 - Coveralls

@MVrachev
Copy link
Collaborator Author

Replaced by #1624.

@MVrachev MVrachev closed this Oct 19, 2021
@MVrachev MVrachev deleted the black-on-tests branch October 27, 2021 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants