lucenenet icon indicating copy to clipboard operation
lucenenet copied to clipboard

Make assertions into a 1st class feature in the test framework

Open NightOwl888 opened this issue 3 years ago • 3 comments

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.

NightOwl888 avatar Jul 07 '20 12:07 NightOwl888

Before putting any more work into nunit, I'd recommend giving xunit a go

I've always preferred it over nunit (and mstest)

TheBeardedLlama avatar Nov 05 '20 09:11 TheBeardedLlama

The Lucene test framework 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 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.

I asked the NUnit team, 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 and 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 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

NightOwl888 avatar Nov 05 '20 14:11 NightOwl888

Just an update on our NUnit integration.

We added the repeatable randomized testing functionality in #547, so now it is much easier to debug random tests. This was done primarily by implementing our own [TestFixture] attribute that enables running the functionality. This attribute is placed within LuceneTestCase so even if any test file has a using NUnit.Framework at the top, the compiler will choose our [TestFixture] instead of the one in NUnit as long as it is a subclass of LuceneTestCase.

The [SuppressCodecs] attribute functions inside of the TestRuleSetupAndRestoreClassEnv, which is executed by NUnit when it calls our LuceneTestCase methods that are decorated with the [OneTimeSetup] and [OneTimeTearDown] attributes. Unfortunately, this means that a user can disable this functionality by overriding these methods without calling the base class (we probably should address that).

We also added [AwaitsFix], [Nightly], [Weekly], and [Slow] attributes so we can classify tests accordingly. These attributes extend NUnit interfaces so NUnit will execute them. Some of them still have issues because they run the most time consuming bits of the test before the attribute has a chance to run. See #739.

We are still missing the [BadApple] attribute, which we don't really have a use for. This was for putting the tests into a "vault" in Jenkins if they fail frequently.

The [SuppressTempFileChecks] attribute still does not function, which we should address at some point. See: #898.

So, we are very much integrated with NUnit at this point, and I am not sure there is another test framework that can support all of this functionality (at least not without making our own fork of it).

That said, NUnit 4 has already shipped and it is not yet known how much work is involved with supporting it.

NightOwl888 avatar Jan 19 '24 08:01 NightOwl888