[GitHub] [lucenenet] NightOwl888 opened a new issue #306: Make assertions into a 1st class feature in the test framework

[GitBox <gi...@apache.org>]

NightOwl888 opened a new issue #306:
URL: https://github.com/apache/lucenenet/issues/306


   We recently discovered that the assertions provided by NUnit do not provide any support for primitive types and in those cases will default to `object` which causes boxing/unboxing when doing assertions. This can be a huge drain on performance when this operation is done in a tight loop. 
   
   It is clear that end users will likely run into similar issues with testing that we did. While we have addressed the problem, we did so in 2 assertions types that are both internal. We need to create a public API for assertions that end users of the test framework can use.
   
   - Missing overloads need to be added to support all of the operations that NUnit does
   - Custom asserts that we created need to be expanded to include all primitive types
   - A logical way to make these asserts "native" in `LuceneTestCase` and all of its subclasses needs to be devised
   
   In Lucene, `LuceneTestCase` simply subclassed `Assert`. However, due to a difference in naming conventions between NUnit and JUnit, this would seem odd. For example, instead of `AssertEquals`, the NUnit team named their method `AreEqual`. While this isn't such a problem, it would be inconsistent with all of the other custom assertions in the test framework (i.e. `AssertAnalyzesTo`).
   
   In addition, API documentation should be provided for each of the assertions.
   
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [lucenenet] NightOwl888 commented on issue #306: Make assertions into a 1st class feature in the test framework

[GitBox <gi...@apache.org>]

NightOwl888 commented on issue #306:
URL: https://github.com/apache/lucenenet/issues/306#issuecomment-722408852


   The Lucene [test framework](https://www.nuget.org/packages/Lucene.Net.TestFramework) is unique in that it is a component that is meant for use by end users as well as our own tests. I did some research, and I cannot find another example of this on the .NET platform - it is unprecedented. In Java, the test framework uses a custom runner called [Randomized Testing](https://github.com/randomizedtesting/randomizedtesting) that is built on top of JUnit using its extensibility. The framework is inheritance-based, uses its own random seed functionality, and has its own attributes it needs to scan for during the test life cycle. Putting in enough functionality to actually debug the random tests (by outputting the random seed that was used during a failure so it can be plugged back in to debug) is a problem that still [hasn't been completely solved yet](https://github.com/apache/lucenenet/issues/288).
   
   I [asked the NUnit team](https://github.com/nunit/nunit/issues/3405), and they don't currently support the extensibility points to build your own test runner. So, in the spirit of not re-inventing the wheel we are trying to make do with only NUnit's features out of the box. Not all of the features of the test framework are supported this way (such as running the tests in a random order), but I believe we have enough of them to get by without our own test runner. NUnit is the only framework that comes close to being able to support an inheritance-based framework without a ton of research and tradeoffs.
   
   In case you haven't been following along for the past few years, @conniey worked on converting to xUnit for several months back in 2016-2017 and ultimately came to the conclusion that it wasn't possible. But I believe she was trying to take advantage of xUnit's parallel features.
   
   In 2019, we tried converting the test framework to both [xUnit](https://github.com/apache/lucenenet/tree/6ea55d1bcacd9322f10b696e3eee4462905a1479/src/Lucene.Net.Tests.TestFramework.xUnit) and [MSTest](https://github.com/apache/lucenenet/tree/6ea55d1bcacd9322f10b696e3eee4462905a1479/src/Lucene.Net.Tests.TestFramework.MSTest) in addition to NUnit.
   
   MSTest was completely impossible to support due to the fact that it doesn't scan for its own attributes in base classes. Maybe that has changed, but it wasn't possible in 2019.
   
   xUnit forces you to inject state into the constructor in order to get any control over what would ordinarily be `[BeforeClass]` and `[AfterClass]` attributes. This puts an unnecessary burden on end users who then have to pass your state object in order to inherit your base class. I did determine that it may be possible to make our tests run with xUnit by turning off its parallel feature and it may even be possible to support parallel testing by refactoring several of Lucene's classes to inject state rather than relying on static variables. However, I am not sure if all of the test framework features we need could be supported. xUnit wasn't designed with an inheritance-based test framework that is meant for 3rd parties in mind, and you have to manually do a lot of the things that are included right out of the box with NUnit, such as scanning for custom-made attributes.
   
   Ultimately, xUnit will be a lot more work to support than NUnit, and the fact that xUnit doesn't support an inheritance-based model that "just works" without forcing end users to override your constructor makes it less desirable than NUnit.
   
   So, after over a year of research and trial and err, we determined that:
   
   1. For the short term at least through to the 4.8.0 release, NUnit is our only viable choice of the top 3 test frameworks
   2. For the long term, it would be better to [port Randomized Testing](https://github.com/apache/lucenenet/issues/288) so we can fully support debugging random tests, but we would have to work closely with the NUnit team to ensure there is enough extensibility to support it


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [lucenenet] TheBeardedLlama commented on issue #306: Make assertions into a 1st class feature in the test framework

[GitBox <gi...@apache.org>]

TheBeardedLlama commented on issue #306:
URL: https://github.com/apache/lucenenet/issues/306#issuecomment-722262837


   Before putting any more work into nunit, I'd recommend giving xunit a go
   
   I've always preferred it over nunit (and mstest)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org