make _CACHE thread-safe #123
Conversation
Codecov Report
@@ Coverage Diff @@
## master #123 +/- ##
==========================================
+ Coverage 96.87% 97.71% +0.84%
==========================================
Files 6 6
Lines 224 219 -5
==========================================
- Hits 217 214 -3
+ Misses 7 5 -2
Continue to review full report at Codecov.
|
| global _CACHE[:PKGS][pkg] = ver | ||
| lock(_CACHE_LOCK) do | ||
| if isempty(_CACHE[:PKGS]) | ||
| for (pkg, ver) in Pkg.installed() |
There was a problem hiding this comment.
Hmm, I think we should have a failing test cases that's fixed by this change? I wonder if we should mock the Pkg.installed() call in order to write an initially failing test for the non-threadsafe version? @oxinabox Do you have any opinions/suggestions on this?
There was a problem hiding this comment.
I agree a test would nice. But I'm not sure how to write one in this case.
There was a problem hiding this comment.
I can't think how to do this reliably.
Because the scheduler never promises to run things concurrently
Race conditions are basically impossible to unit test. So I do not think this is required. |
|
Bump :) |
|
Thanks! |
The global variable
_CACHEis not thread-safe. This PR should fix that.