Fix pop timer crash #720
Merged
Fix pop timer crash #720
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
Test was flaky due to expensive autospecing
Askaholic
force-pushed
the
fix-pop-timer-crash
branch
from
b8e524d
to
05f5419
Compare
cleborys
approved these changes
Feb 14, 2021
| def failed_matching_attempts(self) -> List[int]: | ||
| """Used for logging so returning a different type here is fine""" | ||
| return [search.failed_matching_attempts for search in self.searches] | ||
| def failed_matching_attempts(self) -> int: |
There was a problem hiding this comment.
may nbr_of_failed_matching_attempts is more descriptive (and would have made this more obvious).
There was a problem hiding this comment.
I don't think the naming is a problem, but rather that returning a list here was just a bad idea on my part :P. At the time it wasn't used and I wasn't sure what to return here. I'm gonna add it to the unit tests now.
Askaholic
force-pushed
the
fix-pop-timer-crash
branch
from
05f5419
to
c73c866
Compare
Askaholic
force-pushed
the
fix-pop-timer-crash
branch
from
c73c866
to
d1fe598
Compare
Askaholic
force-pushed
the
fix-pop-timer-crash
branch
from
d1fe598
to
32bfe1c
Compare
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.
It was not directly the fault of the pop timer, but rather an exception that could sometimes occur during random newbie matching.
CombinedSearchobjects would return a list forfailed_matching_attemptsinstead of an integer. This has been fixed and I made sure to add an integration test that covers newbie matching with two solo queue-ers who were placed together by the team creation code. I also wrapped the pop timer loop contents in a try block, since this sort of error should really only be failing the current matching attempt, and not crashing the entire timer loop.