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

profile| Adds (opt-inable) profile-heatmap generation output to tox test-runs #3503

Merged

Conversation

doublethefish
Copy link
Contributor

@doublethefish doublethefish commented Apr 21, 2020

Steps

  • Add yourself to CONTRIBUTORS if you are a new contributor.
  • (n/a) Add a ChangeLog entry describing what your PR does.
  • (n/a) If it's a new feature or an important bug fix, add a What's New entry in doc/whatsnew/<current release.rst>.
  • Write a good description on what the PR does.

Description

Fairly straight-forward change enabling the creation heatgraphs for test-runs, when and as needed.

We can use tox -epy38 -- --profile-svg to enable this functionality now that we have the pytest-profiling dependency added to tox.

Partly addresses #1954. Split out from the original PR for #3473.

Type of Changes

Type
🔨 Refactoring

Related Issue

@coveralls
Copy link

Coverage Status

Coverage remained the same at 90.454% when pulling 6208861 on doublethefish:profile/heatmaps_for_testruns into 67d0572 on PyCQA:master.

@coveralls
Copy link

coveralls commented Apr 21, 2020

Coverage Status

Coverage remained the same at 90.442% when pulling 1e21a0d on doublethefish:profile/heatmaps_for_testruns into 0ea3410 on PyCQA:master.

@PCManticore
Copy link
Contributor

Thanks for the PR. I think this should be either a separate step or controlled via an additional parameter (which we can already pass via posargs). The reason is that running the test suite locally already takes a bit for contributors, but slowing it down wholesale would not make a lot of sense if folks don't work on performance related patches.

@doublethefish
Copy link
Contributor Author

doublethefish commented Apr 22, 2020

I very much agree.

How does the 'postargs' option work? I've not been able to use it for stuff like this, passing --profile-svg or test-paths to tox and I've not found the docs very useful. If we can add an arg switch to enable it for the test-runs that would be ideal as being able to get perf info for the tests is really useful.

Adding a new step means code-duplication, which will make the tox config more complex and I feel that we should also avoid that. What I suggest is just taking PR #3498 for now and if we can get post-args to work for this later then do so.

Edit: I reread the tox docs and updated the description. This a much more maintainable patch now IMO

We can use `tox -epy38 -- --profile-svg` to enable this functionality
now that we have the pytest-profile dependency.
@doublethefish doublethefish changed the title profile| Adds profile-heatmap generation output to tox test-runs profile| Adds (opt-inable) profile-heatmap generation output to tox test-runs Apr 22, 2020
@PCManticore
Copy link
Contributor

Nice, thanks a lot!

@PCManticore PCManticore merged commit 0ccf1bc into pylint-dev:master Apr 23, 2020
@doublethefish doublethefish deleted the profile/heatmaps_for_testruns branch April 23, 2020 10:36
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.

3 participants