Prevent users from overriding test framework setup/teardown functionality

[NightOwl888]

Is there an existing issue for this?

• I have searched the existing issues

Task description

In Lucene, it was not possible to override the BeforeClass, AfterClass, Before, or After methods. This was enforced by the compiler by declaring the methods static.

Correction: Lucene made Before and After methods instance methods that were virtual.

The same logic still applies for taking ownership of them, though. We should provide custom [SetUp] and [TearDown] attributes for end users and create non-virtual methods that are called directly from NUnit that scan for them and direct the flow of execution to the virtual SetUp() and TearDown() methods and elsewhere to fix the call order. The following rules should be enforced with Roslyn code analyzers:

1. The user should get a failure if the attempt to use [NUnit.Framework.SetUp] or [NUnit.Framework.TearDown] attributes directly and should be instructed to use the [SetUp] and [TearDown] attributes in LuceneTestCase.
2. The user should get a failure if they override SetUp() or TearDown() if they fail to call base.SetUp() or base.TearDown().
3. The user should get a warning if they override SetUp() or TearDown() if they don't call base.SetUp() prior to their code or base.TearDown() after their code. This rule may be overridden for special cases.

In Lucene.NET, we have defined these methods as virtual instance methods. This because NUnit calls the methods that implement the [SetUp], [OneTimeSetUp], [TearDown] and [OneTimeTearDown] attributes in the reverse order with respect to inheritance as JUnit and it was the quickest way to fix the calling order issue (and in turn, we were able to debug the tests sooner). For OneTimeSetUp(), the LuceneTestCase base class needs to be called first, so this approach allows the most derived class to call the root base class by using base.OneTimeSetUp() and cascading it through each level of inheritance.

While this works for our internal purposes, it allows users to override the methods and not call the base class. This can disable critical functionality for setting up or tearing down the test framework, which ideally the user would not be able to do.

I have analyzed NUnit thoroughly (in the past). Since these methods are called during the execution phase of the test run (which NUnit gives us no control over), there is no way to change the order. At least not without creating our own fork of NUnit. However, I thought of a way we might be able to fix this post-NUnit.

1. Declare the methods SetUp(), OneTimeSetUp(), TearDown() and OneTimeTearDown() as static as they were in Lucene using the original NUnit [SetUp], [OneTimeSetUp], [TearDown] and [OneTimeTearDown] attributes. These should be renamed beginning with a double underscore and be given the EditorBrowsable(EditorBrowsableState.Never)] attribute.
2. Create a set of [SetUp], [OneTimeSetUp], [TearDown] and [OneTimeTearDown] attributes that are nested inside of the LuceneTestCase class for users. These attributes will be owned by our test framework and unknown to NUnit. We can add an Order property that can optionally be set to define which order to execute them in when 2 or more are defined in the same class. This allows users to use attributes with names they are familiar with and as long as they don't fully qualify them, they will be our attributes rather than NUnit's. We use this approach already on [TestFixtureAttribute].
3. (optional) Create a set of [BeforeSetUp], [BeforeOneTimeSetUp], [AfterTearDown] and [AfterOneTimeTearDown] attributes that give the user the option to execute code prior to setup and after teardown. Note that using custom attributes based off of NUnit interfaces may be an alternative to this (basically, if you need to run something before SetUp() or OneTimeSetUp() or after TearDown() or OneTimeTearDown(), create an attribute to do it and decorate either the class or the test, as appropriate).
4. When NUnit calls the SetUp(), OneTimeSetUp(), TearDown() and OneTimeTearDown() methods in LuceneTestCase, find the attributes for the current class and all classes that it inherits, order them appropriately, and execute them.

Pros

1. Allows subclasses to use either static or instance methods.
2. Closely aligns with Lucene.
3. Creates a layer of indirection that we can use to potentially address other issues, such as [Slow] attribute runs the code in BeforeClass() and AfterClass() even when the tests:slow property is set to false #739. It will be straightforward to conditionally stop the call to the actual setup and tear down methods based on a custom attribute if we are calling these methods instead of NUnit.

Cons

1. Less transparency in the calling order, since inheritance is easy to understand.
2. The compiler displays a warning when there is a static method with the same name in a base class and a derived class.

Note that we should analyze how the Reflection calls are done in NUnit so we can optimize. It might make sense to piggyback off of the test discovery phase ([TestFixtureAttribute]) to store the metadata about where the attributes are located and even the appropriate order to call them in, which can be stored in the RandomizedContext class instance (which only applies to the currently running test).

After it is implemented, we should also revert the existing SetUp(), OneTimeSetUp(), TearDown() and OneTimeTearDown() methods of subclasses to the way they were in Lucene. Most of them were static, and several of them had names other than these (although, we should stick with these naming conventions instead of the before/after conventions that were used in Java - see #1016).

The way this is supposed to work is that if a user subclasses LuceneTestCase, it will opt in to all of the features of the test framework, so we can rewrite the rules on how it functions that are differently than NUnit's normal rules. This is an all or nothing proposition, the user can only opt out by not subclassing LuceneTestCase. If a test class does not subclass LuceneTestCase, it will just be a "normal" NUnit test and won't have access to any of the test framework functionality.

Maybe someday when we have it stable enough, we can create a separate RandomizedTesting package that actually does most of what the randomized-testing project does (see #257). When that is the case, there will be a base class of LuceneTestCase (probably named RandomizedTestCase) and the generic extensions to NUnit (such as these custom attributes) will move up a level to RandomizedTestCase so they can be reused in by other projects to implement randomized testing.

[paulirwin]

Related: #1088. This item (1087) should probably be done first.

We should make sure that is considered when doing this, but I think this will potentially help the #1088 implementation.