Test review Core J-S, #259

[paulirwin]

• You've read the Contributor Guide and Code of Conduct.
• You've included unit or integration tests for your change, where applicable.
• You've included inline docs for your change, where applicable.
• There's an open issue for the PR that you are making. If you'd like to propose a change, please open an issue to discuss the change or find an existing issue.

Test review for core namespaces J-S

Partial work for #259

Description

• Test review and code cleanup for J-S tests
• Formatting updated to match upstream Java code for easier line-by-line comparison
• Miscellaneous test performance improvements (variables -> consts, some singleton instances added)
• Note: some removals of unused outerInstance for inner/anonymous classes that did not use it. Reviewed latest Lucene Java code to ensure this capturing isn't use so that it won't be burdensome work to re-add later. Many cases were actually static inner classes in Java so this was incorrect anyways.

[NightOwl888]

Nice work! Your help on this issue is much appreciated.

There are some issues to correct, including one you specifically mentioned on Slack.

There are also several issues to open that are tangential to this PR, but go beyond its scope. These are some of the issues to look for in #259. We will need new GitHub issues for those.

I also noticed we don't have tests for the following types in the Support folder:

• IndexableFieldExtensions
• ByteArrayOutputStream - Patch to provide ToString() output for Stream.
• StreamExtensions - These methods were copied from Apache Harmony, and there are tests for them that didn't get ported over. Need to drill down in the code to find the location where these originated from.
• ReferenceContext<T> - We added this to make it possible to use a using block instead of Acquire() with a try finally around Release(). We should convert this to a ref struct, so it doesn't cause a heap allocation to use. Ref structs cannot implement IDisposable, but the compiler allows a using block if Dispose() exists. Note that ReferenceManagerExtensions can be tested along with it.
• LimitedConcurrencyLevelTaskScheduler - We originally grabbed this from MSDN. We might be able to use tests from Apache Harmony to test this.
• ReentrantLock - There are tests in Apache Harmony for this.
• CastTo<T>
• ExceptionExtensions - Apache Harmony should have tests for these.
• ListExtensions
• SystemConsole - .NET should have some tests we can use for this.
• Arrays - Apache Harmony should have tests we can use for this.
• AssemblyUtils
• CollectionExtensions - We can base the tests off of Apache Harmony tests,, but we will need to check ISet<T>, JCG.List<T>, SCG.List<T> and a collection that does not implement ISet<T> or IList<T> (the slow path). All should produce the same result.
• Collections - There should be tests in Apache Harmony for this.
• ConcurrentDictionaryWrapper<TKey, TValue> - This is no longer in use, we can probably eliminate it along with the DictionaryExtensions.AsConcurrent() extension method. It may come in handy for a future version of Lucene, though.
• ConcurrentHashSet<T> - Apache Harmony should have tests for this. Note that not all ISet<T> members are implemented. If they were, we could potentially move this to J2N.
• ConcurrentSet<T> - This is a concurrent wrapper for any ISet<T>. It should pass the same tests as ConcurrentHashSet<T>. It is mainly used for wrapping LinkedHashSet<T>. If we had a ConcurrentLinkedHashSet<T> in J2N, we could eliminate this class.
• DictionaryExtensions

There are a few others, but these are the most important ones. These tests don't have to be part of this PR, but for this review each one should have a new GitHub issue.

FYI - After reviewing the RandomExtensions class, I noticed that we are calling the wrong implementation when using J2N.Randomizer, so I opened NightOwl888/RandomizedTesting#4.

[paulirwin]

Created issue to track missing Support namespace unit tests: #920

[NightOwl888]

Nice work! This is good to merge.

No objections if you wish to roll in the removal of ConcurrentDictionaryWrapper<TKey, TValue> and friends before it is merged, though. See #920 (comment).