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

chore(mypy): fix cache cleanup #15072

Merged
merged 2 commits into from
May 29, 2024
Merged

Conversation

ahiuchingau
Copy link
Contributor

Overview

We haven't been properly clearing the mypy cache for some python directories when we do make clean-py because we were using the wrong paths.

It seems like we don't clear pytest caches in most directories in the cleanup rules, is that intentional?

@ahiuchingau ahiuchingau requested review from a team as code owners May 2, 2024 16:18
Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

Do we have to? It does tend to slow things down. Maybe we can make clean-extra targets if we really want to?

@ahiuchingau
Copy link
Contributor Author

Do we have to? It does tend to slow things down. Maybe we can make clean-extra targets if we really want to?

Do you mean the pytest cache or mypy or both? Do you propose that I move the mypy cache and add pytest cache to a different target (makes sense and I'm very ok with that)?

For context, I noticed this only because I was switching between branches that have different pipenv requirements and for some reason mypy was not behaving unless I manually removed the cache first. So I thought not clearing the cache properly was the problem. But now I can't seem to reproduce the same issue I was having so I'm stumped.

@sfoster1
Copy link
Member

sfoster1 commented May 3, 2024

Do we have to? It does tend to slow things down. Maybe we can make clean-extra targets if we really want to?

Do you mean the pytest cache or mypy or both? Do you propose that I move the mypy cache and add pytest cache to a different target (makes sense and I'm very ok with that)?

For context, I noticed this only because I was switching between branches that have different pipenv requirements and for some reason mypy was not behaving unless I manually removed the cache first. So I thought not clearing the cache properly was the problem. But now I can't seem to reproduce the same issue I was having so I'm stumped.

Yeah the thing is that running mypy over and over with no cache is slower, and we always clean when we build, so you'd end up with slow behavior when you do normal dev processes.

That said we definitely shouldn't have wrong paths in there, i wonder if having like clean-build removing build/ and such and clean: removing all caches would be a good idea. then the build targets depend on clean-build and clean is only ever run by humans.

Copy link

codecov bot commented May 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.43%. Comparing base (8f50b08) to head (f0bceb9).
Report is 234 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             edge   #15072       +/-   ##
===========================================
+ Coverage   67.50%   92.43%   +24.93%     
===========================================
  Files        2521       77     -2444     
  Lines       72090     1283    -70807     
  Branches     9311        0     -9311     
===========================================
- Hits        48666     1186    -47480     
+ Misses      21228       97    -21131     
+ Partials     2196        0     -2196     
Flag Coverage Δ
api ?
app ?
components ?
g-code-testing 92.43% <ø> (ø)
hardware ?
hardware-testing ?
labware-library ?
notify-server ?
ot3-gravimetric-test ?
protocol-designer ?
react-api-client ?
robot-server ?
shared-data ?
step-generation ?
system-server ?
update-server ?
usb-bridge ?

Flags with carried forward coverage won't be shown. Click here to find out more.

see 2444 files with indirect coverage changes

Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

looks good, ty!

@ahiuchingau ahiuchingau merged commit 538b774 into edge May 29, 2024
60 checks passed
@ahiuchingau ahiuchingau deleted the chore_clear-mypy-cache-properly branch May 29, 2024 17:14
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.

None yet

2 participants