-
Notifications
You must be signed in to change notification settings - Fork 642
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 ConcurrentHashSet, #1117 #1128
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
I have been considering phasing out the SystemTypesHelpers
class (or at least most of the extension methods in it), so ideally any new tests we add would not have any dependencies on those extension methods (.add()
, .contains()
, .size()
, etc.).
If we do keep them, they will definitely need a review. Some of the methods perform poorly or have better alternatives (.toString()
, removeAll()
, retainAll()
, and ToByteArray()
to name a few). The .append()
overloads are generally there to allow culture invariant numbers to be appended, since .NET doesn't have that built-in. However, I am adding optional parameters to allow passing format
and IFormatProvider
on OpenStringBuilder
in J2N. So, we should probably hold off on the review/removal of SystemTypesHelpers
until after J2N has a build with OpenStringBuilder
where those can be replaced directly by swapping in OpenStringBuilder
for StringBuilder
.
Add support unit tests for ConcurrentHashSet
Fixes #1117
Description
This ports tests from Harmony for
Collections.synchronizedSet
andConcurrentHashMap
and adapts them to ourConcurrentHashSet
.This also adds nullable reference type checking to ConcurrentHashSet, which exposed some nullability issues. Given that our current use of this type is for values that should never be null (or would otherwise throw an NRE upon use as null is not checked-for), this adds a generic type constraint of
notnull
to ensure that it cannot be used with nullable reference types. Work would need to be done (under proper unit test coverage) to make this null-friendly if that is needed in the future.