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

Use STDIN project in test_projects to ensure it runs quickly #2575

Merged
merged 1 commit into from Oct 30, 2021

Conversation

nipunn1313
Copy link
Contributor

Fixes @iChard's comment in #2555 (comment)
Runs in 0.4 seconds rather than 8 seconds previously (on my machine).

Checklist - did you ...

(none needed)

  • Add a CHANGELOG entry if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation?

Existing test was actually running a full black-primer
run which could be slow. This goes from 8 seconds to
0.4 seconds on my machine.

Needed to move to top level scope to leverage the caplog
feature of pytest in order to test that the command line
was parsing the bogus arguments and dumping to stderr.
@ichard26
Copy link
Collaborator

You pinged the wrong person fyi :p hopefully they aren't bothered

@ichard26 ichard26 added C: maintenance Related to project maintenance, e.g. CI, testing, policy changes, releases skip news Pull requests that don't need a changelog entry. labels Oct 29, 2021
@nipunn1313
Copy link
Contributor Author

🤦

Copy link
Collaborator

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still surprisingly slow at ~1.2s on my machine but this is good enough (and won't slow build + test cycles by any reasonable margin). Thank you for the quick turnaround!

FWIW while it's great you're working on black-primer, I'm currently writing a semi-replacement for the development tool: https://github.com/ichard26/diff-shades. Fundamentally it takes the idea of black-primer and extends it with whatever great ideas mypy-primer added (theirs was inspired by ours), notably diffing support between runs. It'll take a long time before diff-shades is at feature and quality parity, but I thought I'd let you know.

Basically for now if there's any ideas you have for primer but that seem too complicated to implement in primer I'd be happy to investigate if it'd be a good fit for diff-shades.

Copy link
Collaborator

@cooperlees cooperlees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Speed is good. I did have to take a few passes tho to see how this worked. Like @ichard26 I was amazed that pytest log magic and click's magic get along.

@cooperlees cooperlees merged commit 92eeacc into psf:main Oct 30, 2021
@nipunn1313
Copy link
Contributor Author

Speed is good. I did have to take a few passes tho to see how this worked. Like @ichard26 I was amazed that pytest log magic and click's magic get along.

I was too. I actually couldn't get it to work any other way. click.stderr and click.output didn't have the relevant information. I was stumped momentarily until I figured out that pytest was eating the log lines.

Basically for now if there's any ideas you have for primer but that seem too complicated to implement in primer I'd be happy to investigate if it'd be a good fit for diff-shades.

Good to know! Thank you. That would be really valuable! I've used mypy-primer and the diff mode is fantastic. If implemented in diff-shades then you can get valuable feedback running it against large projects that don't fully use black yet (eg django!), especially when trying to implement a stability policy.

JelleZijlstra pushed a commit that referenced this pull request Nov 16, 2021
Existing test was actually running a full black-primer
run which could be slow. This goes from 8 seconds to
0.4 seconds on my machine.

Needed to move to top level scope to leverage the caplog
feature of pytest in order to test that the command line
was parsing the bogus arguments and dumping to stderr.
@nipunn1313 nipunn1313 deleted the cleanup_test branch November 16, 2021 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: maintenance Related to project maintenance, e.g. CI, testing, policy changes, releases skip news Pull requests that don't need a changelog entry.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants