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 ConcurrentSet #1129

Merged
merged 6 commits into from
Feb 21, 2025
Merged

Conversation

paulirwin
Copy link
Contributor

  • 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.

Adds support unit tests for ConcurrentSet

Fixes #1118

Description

As noted in #1118, this type should pass the same tests as ConcurrentSet (apart from the constructor differences). So this PR takes the tests created in #1117 and refactors them out into a base class that can be used for both ConcurrentHashSet and ConcurrentSet test fixtures. The two protected abstract methods are factory methods to create the specific implementation being tested. The two constructor tests that are specific to ConcurrentHashSet only exist on TestConcurrentHashSet.

Additionally, this PR removes a few more usages of SystemTypesHelpers in those shared tests.

@paulirwin paulirwin added the notes:improvement An enhancement to an existing feature label Feb 16, 2025
Copy link
Contributor

@NightOwl888 NightOwl888 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ran the tests on Azure DevOps. There are failures now on every run. Most of these are due to #1123. Could you have a look at that?

This one is new, but it might be a one-off:

  Passed Lucene.Net.Store.TestDirectory.TestLUCENENET521 [100 ms]
  Passed Lucene.Net.Store.TestDirectory.TestNotDirectory [3 ms]
  Passed Lucene.Net.Store.TestDirectory.TestRAMDirectoryFilter [< 1 ms]
[ERROR] OneTimeTearDown: An exception occurred during CleanupTemporaryFiles() in Lucene.Net.Store.TestDirectory:
System.IO.IOException: Could not remove the following files (in the order of attempts):
   C:\Users\VssAdministrator\AppData\Local\Temp\LuceneTemp\LUCENENET521-ujwvk5d1\_0.cfs
   C:\Users\VssAdministrator\AppData\Local\Temp\LuceneTemp\LUCENENET521-ujwvk5d1\_0.cfx
   C:\Users\VssAdministrator\AppData\Local\Temp\LuceneTemp\LUCENENET521-ujwvk5d1

   at Lucene.Net.Util.TestUtil.Rm(FileSystemInfo[] locations) in /_/src/Lucene.Net.TestFramework/Util/TestUtil.cs:line 66
   at Lucene.Net.Util.LuceneTestCase.CleanupTemporaryFiles() in /_/src/Lucene.Net.TestFramework/Util/LuceneTestCase.cs:line 3172
   at Lucene.Net.Util.LuceneTestCase.OneTimeTearDown() in /_/src/Lucene.Net.TestFramework/Util/LuceneTestCase.cs:line 1039
  Passed Lucene.Net.Store.TestDirectory.TestThreadSafety [7 s]
  Passed Lucene.Net.Store.TestFileSwitchDirectory.TestBasic [164 ms]
  Passed Lucene.Net.Store.TestFileSwitchDirectory.TestCompoundFileAppendTwice [34 ms]

Copy link
Contributor

@NightOwl888 NightOwl888 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the test failures should now be resolved by #1131, this looks good to merge.

@paulirwin paulirwin merged commit 77a661d into apache:master Feb 21, 2025
275 checks passed
@paulirwin paulirwin deleted the issue/1118 branch February 21, 2025 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notes:improvement An enhancement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Support unit tests for ConcurrentSet
2 participants