Add a coverage test for pantsd garbage collection, and fix type error #9271
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stuhood
force-pushed
the
stuhood/coverage-test-for-store-gc-service
branch
from
cb61011 to
abc3d5a
Compare
Eric-Arellano
approved these changes
Mar 11, 2020
| # Ensure that the thread runs successfully for long enough to have run each step at least once. | ||
| # TODO: This is a coverage test: although it could examine the internal details of the service | ||
| # to validate correctness, we don't do that yet. | ||
| time.sleep(interval_secs * 10) |
There was a problem hiding this comment.
Have you tried running this multiple times to test for flakiness? Note that you'd need to use the V1 test runner because of V2's caching.
There was a problem hiding this comment.
I have not, but I believe that the deltas here are sufficient to avoid flakes. If not, they should be very easy to tune.
…erSession, but getting a Scheduler.
stuhood
force-pushed
the
stuhood/coverage-test-for-store-gc-service
branch
from
abc3d5a to
28fb234
Compare
stuhood
added a commit
that referenced
this pull request
Mar 12, 2020
#9271) ### Problem `StoreGCService` was expecting a `SchedulerSession`, but getting a `Scheduler`. This meant that if `pantsd` ran long enough (the default value for "long enough" was too long to ever be caught in tests), it would crash with a failure to call `lease_files_in_graph` due to the method signature change in #9015. ### Solution Although just adding enough type hints here would hypothetically be enough for mypy to catch this (?), additionally added a coverage test for `StoreGCService`. ### Result `pantsd` stays up as long as it should.
wisechengyi
pushed a commit
that referenced
this pull request
May 4, 2020
#9271) ### Problem `StoreGCService` was expecting a `SchedulerSession`, but getting a `Scheduler`. This meant that if `pantsd` ran long enough (the default value for "long enough" was too long to ever be caught in tests), it would crash with a failure to call `lease_files_in_graph` due to the method signature change in #9015. ### Solution Although just adding enough type hints here would hypothetically be enough for mypy to catch this (?), additionally added a coverage test for `StoreGCService`. ### Result `pantsd` stays up as long as it should.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
StoreGCServicewas expecting aSchedulerSession, but getting aScheduler. This meant that ifpantsdran long enough (the default value for "long enough" was too long to ever be caught in tests), it would crash with a failure to calllease_files_in_graphdue to the method signature change in #9015.Solution
Although just adding enough type hints here would hypothetically be enough for mypy to catch this (?), additionally added a coverage test for
StoreGCService.Result
pantsdstays up as long as it should.