Add expiration argument back to GithubIntegration.create_jwt#2439
Add expiration argument back to GithubIntegration.create_jwt#2439s-t-e-v-e-n-k merged 2 commits intoPyGithub:masterfrom
Conversation
4f3913f to
2e82f31
Compare
Codecov ReportBase: 98.77% // Head: 98.77% // Increases project coverage by
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #2439 +/- ##
=======================================
Coverage 98.77% 98.77%
=======================================
Files 117 117
Lines 11674 11677 +3
=======================================
+ Hits 11531 11534 +3
Misses 143 143
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
s-t-e-v-e-n-k
left a comment
There was a problem hiding this comment.
Looking good, just one niggle.
|
|
||
| def testCreateJWTWithExpiration(self): | ||
| self.origin_time = sys.modules["time"].time | ||
| sys.modules["time"].time = lambda: 1550055331.7435968 |
There was a problem hiding this comment.
This feels awkward. Why not use a mock here?
There was a problem hiding this comment.
I don't like it either, just copied from existing testCreateJWT above.
There was a problem hiding this comment.
Then we should fix both places, and I'd rather circle back and fix one, rather than both.
There was a problem hiding this comment.
This is meant to be a quick fix, to get a fix for 1.58.0 out, asap. People are rightly annoyed: #2430 (comment)
Cleaning up test code can be done for 1.59.0.
There was a problem hiding this comment.
I don't have time to cut a release right now, but I'll get this merged.
1172288 to
57e8977
Compare
Fixes #2432.