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

[Infrastructure] Code coverage #277

Open
gyermolenko opened this issue Feb 7, 2019 · 4 comments
Open

[Infrastructure] Code coverage #277

gyermolenko opened this issue Feb 7, 2019 · 4 comments
Labels

Comments

@gyermolenko
Copy link
Contributor

@gyermolenko gyermolenko commented Feb 7, 2019

As far as I know almost always coverage is used to see "test coverage".
For unused branches/variables etc there are linters, dead-code finders (such as vulture or dead).

So I don't think code coverage is important to extent of running run_all.sh script.

I would like to

  • remove run_all.sh
  • collect coverage
    • only for tests
    • only for one python branch (e.g. 3.7) (although it is possible to do coverage run -p and then coverage combine)
@faif
Copy link
Owner

@faif faif commented Feb 8, 2019

What's the reasoning behind running it only for a specific Python version (e.g. 3.7)?

@faif faif added the enhancement label Feb 8, 2019
@gyermolenko
Copy link
Contributor Author

@gyermolenko gyermolenko commented Feb 8, 2019

What's the reasoning behind running it only for a specific Python version

a bit simpler, I guess. But combining is possible, so nevermind.
My biggest concern here is - why run_all.sh at all?

@faif
Copy link
Owner

@faif faif commented Feb 9, 2019

I think that the basic idea was to able to run all scripts with one command. Coverage came later. So if we can simplify it by removing the coverage part since it's handled elsewhere, fine by me.

@gyermolenko
Copy link
Contributor Author

@gyermolenko gyermolenko commented Jan 14, 2020

To update on this issue:

  • run_all.sh was removed
  • since removal of py2 support full coverage can be done without combining reports per test environment

So only some simplifications in tox.ini are required to resolve the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.