Update syntax for Julia v1 #4
Conversation
|
The commit that moved it out was JuliaLang/julia@a0b7a76 |
|
Try removing appveyor and using Travis Windows (just add |
test/runtests.jl
Outdated
| using Test | ||
|
|
||
| @testset "ReadWriteLocks" begin | ||
| include("singlethreaded.jl") |
There was a problem hiding this comment.
What about the "Two-threaded tests" testset?
There was a problem hiding this comment.
you named it 😆 I will just move them into the runtests file 😄
There was a problem hiding this comment.
Oh lol, maybe because they were never actually run on multiple threads?
| @@ -49,7 +39,7 @@ write_lock(rwlock::ReadWriteLock) = rwlock.write_lock | |||
|
|
|||
| function lock!(read_lock::ReadLock) | |||
There was a problem hiding this comment.
This API should be changed to match the AbstractLock API, including the other API functions like trylock.
There was a problem hiding this comment.
You mean also rename lock -> lock! (even though we mutate the rwlock.readers)?
test/singlethreaded.jl
Outdated
| for i = 1:NUM_LOCKS | ||
| @fact lock!(rlock) --> nothing | ||
| @fact rwlock.readers --> i | ||
| @test lock!(rlock) == nothing |
There was a problem hiding this comment.
We don't need to do any tests of the return values of lock and unlock anymore. We can just call the functions.
FactCheck was not as good as Test at handling errors outside of tests.
There was a problem hiding this comment.
hmm, changing all cases of @test [un]lock!(x) == nothing -> [un]lock!(x) causes tests to fail -- we hit code we were not hitting before 🤔
There was a problem hiding this comment.
Push a commit that does that so I can see
There was a problem hiding this comment.
lol okay put them back I guess? Bizarre.
There was a problem hiding this comment.
reverted 🤷♂
test/runtests.jl
Outdated
| using ReadWriteLocks | ||
| using Test | ||
|
|
||
| @testset "ReadWriteLocks" begin |
There was a problem hiding this comment.
Now that Julia 1.2 and 1.3 exist, we should add tests using actual threads. We can talk about that on Slack.
There was a problem hiding this comment.
in case we do this in a follow-up PR, opened #6
Codecov Report
@@ Coverage Diff @@
## master #4 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 1 1
Lines 41 39 -2
=====================================
- Hits 41 39 -2
Continue to review full report at Codecov.
|
This reverts commit b006f13.
ReentrantLockwhat we want here?AbstractLockout of theThreadssubmodule