Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Support unit tests for ReentrantLock #1111

Closed
paulirwin opened this issue Jan 21, 2025 · 3 comments
Closed

Add Support unit tests for ReentrantLock #1111

paulirwin opened this issue Jan 21, 2025 · 3 comments
Assignees

Comments

@paulirwin
Copy link
Contributor

paulirwin commented Jan 21, 2025

There are tests in Apache Harmony for this.

@paulirwin paulirwin changed the title ReentrantLock - There are tests in Apache Harmony for this. Add Support unit tests for ReentrantLock Jan 21, 2025
@paulirwin paulirwin self-assigned this Jan 22, 2025
@paulirwin
Copy link
Contributor Author

I will take this one once #1119 is merged, since it improves the JSR166TestCase base class.

For posterity, the Apache Harmony tests are here: https://github.com/apache/harmony/blob/trunk/classlib/modules/concurrent/src/test/java/ReentrantLockTest.java

@paulirwin
Copy link
Contributor Author

@NightOwl888 I briefly looked into this one, and it appears that it already has a few tests enabled from the Harmony tests: https://github.com/apache/lucenenet/blob/master/src/Lucene.Net.Tests/Support/Threading/ReentrantLockTest.cs

Of course a lot of them are commented out because the ReentrantLock class only seems to implement what is minimally needed. I don't know if it's worth the effort to port the entire hierarchy and behavior of ReentrantLock from Harmony, as that would be very complex and time-consuming, especially if we're not using that functionality beyond what it does today. Let me know if you have specific thoughts on anything to do here, or if this can be closed out.

@NightOwl888
Copy link
Contributor

I agree. The tests added in #940 are sufficient. There is no reason to port any more of ReentrantLock.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants