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

Port RandomizedTesting Test Runner #264

Open
clambertus opened this issue Oct 10, 2019 · 4 comments
Open

Port RandomizedTesting Test Runner #264

clambertus opened this issue Oct 10, 2019 · 4 comments
Labels
pri:low up-for-grabs This issue is open to be worked on by anyone
Milestone

Comments

@clambertus
Copy link

While the test framework can run with the standard NUnit test runner and we have managed to make it work, it is missing the ability to reproduce a random test failure, which is crucial for debugging some of the more complicated random tests. Lucene uses a custom JUnit test runner called RandomizedTesting to accomplish this.

It also includes some other nice features

  1. Ensure when a test fails, the random seeds are included in error messages and logs
  2. Code analysis to ensure the tests are setup properly
  3. Run tests in a random order to ensure they are not dependent upon each other

Preliminary analysis shows that the API for NUnit allows building custom test runners and it is a close enough match to implement the functionality. Most likely, this will require a custom adapter as well, so the test runner can integrate into Visual Studio and dotnet test/mstest.

Do note that without doing some pretty heavy refactoring on the Codec, Directory, and LuceneTestCase classes, it is not possible to run tests in parallel within the same AppDomain because the codecs use a static variable to turn codec impersonation on/off. For now, it would probably be best to run tests serially.

JIRA link - [LUCENENET-627] created by nightowl888
@clambertus
Copy link
Author

I have opened a dialog with the NUnit team, trying to gauge whether we can get them to add the most important features we need, whether it will be entirely up to us to port the test runner and create our own Visual Studio / dotnet test adapters, or somewhere in the middle: nunit/nunit#3405

by nightowl888

@clambertus
Copy link
Author

After updating the test framework to utilize the NUnit.Framework.TestContext.CurrentContext.Random property, I noticed that random test failures now repeat reliably until the next build. Whether or not there is a way to lock in that behavior from one build to the next, this is probably adequate enough to avoid porting the entire RandomizedTesting framework.

That said, it would probably make sense to create a new library with the random generators so they can be shared without a reference to our test framework and all of the dependencies that come with it. We should also migrate the extension methods for the Random class.

by nightowl888

@clambertus
Copy link
Author

Lowering priority of this and removing the 4.8.0 version, as it is not required for the Lucene.NET release. It might still be useful to do this someday, so I am keeping the issue open.

by nightowl888

@NightOwl888
Copy link
Contributor

There may be some compelling reasons that make RandomizedTesting useful for Lucene.NET: https://chatgpt.com/share/677a832d-3424-8005-97d7-1c024c68302d.

There are a few features that Lucene uses that we don't have (such as running tests in random order and varying other parts of the test run). However, one benefit that makes a slightly stronger case for it: It would provide a level of indirection between NUnit's release schedule and Lucene.NET's schedule. In theory, we could roll out a new version of RandomizedTesting to keep up with NUnit changes without forcing us to also update Lucene.NET.

While at this point I still think it is not required for the release of Lucene.NET 4.8.0, even with the latest update on #1017, it does mean that everything that we import from RandomizedTesting should be made internal and not available for end users. This is a must if we are going to expose it publicly later in another library. Ideally, we would move everything into the RandomizedTesting namespace and physically separate it from the test framework to make it less of a burden to make public later.

RandomizedTesting in Java is implemented as a test runner. Based on my experience with setting up tests on Xamarin, I think if we did do a port of it, it should be implemented as a test runner that has a separate test adapter package (unfortunately, a test adapter is required for a custom runner, so we would need both). Separating them allows for a broader range of use cases than just plugging it into .NET Core or.NET Framework (such as building a custom console app to extend the functionality) or using it in some past or future runtime.

Having our own test adapter is the only way we can take charge of NUnit instead of having NUnit drive everything. I don't think there is anything we can extend in the execution layer of NUnit without a test adapter, which is the only way we would be able to change how threads and/or tasks are implemented or to change the order of test execution in a repeatable random way.

Also note that there already is a NuGet package ID for RandomizedTesting and a project in the RandomizedTesting repository. It wouldn't be very difficult to configure it to release one package at a time each with their own version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pri:low up-for-grabs This issue is open to be worked on by anyone
Projects
None yet
Development

No branches or pull requests

2 participants