dodexahedron
commented
10 months ago The NUnit documentation is fragmented, incomplete, and at various levels of outdated, where things relevant to this issue are concerned.
However, assuming that the documentation about attributes still applies, it looks like, at minimum, we should properly decorate all test classes and methods with appropriate attributes, and likely should also have SetUp and TearDown labeled methods in each test fixture. This is because, according to the available documentation, attribute inheritance is neither guaranteed nor intuitive for most attributes.
This isn't a difficult change, by itself. It just touches nearly every file.
I'll do that before continuing with other considerations.
tznind
All unit tests in the test project inherit from a base
Testsclass.While this, in and of itself, is not a problem, some of the static members of that class have high potential for causing subtle problems in testing - some of which are only becoming apparent as the unit tests are being updated to NUnit 4.0 and also being re-factored to split, enhance, expand, and formalize the tests.
While this issue was prompted initially by concerns raised in #29, I think there's value in at least taking a good look at this class to be sure it does what's expected. It almost deserves some tests of its own. 😅
Some thoughts, none of which are necessarily known to be directly related to any existing problems:
While individual TestFixtures are supposed to be executed in a separate AppDomain, I am not clear on if each fixture gets its own AppDomain (and I think they do not). If that is true, and they do not each get a fresh AppDomain, then that is actually a base issue that needs to be addressed, anyway (and may be impacting #29 as well), because that means that static items are dangerous unless they follow very tight rules.
In particular, any static declared in the test project, TGD, or even TG, that is shared by multiple test fixtures (or even test methods, when relevant) that are not pure or readonly (all the way down the object graph, including any side effects of constructors of elements or properties created when those items are created), and which return instances of reference types or return references to or boxed instances of value types, that a test or other item may then mutate, in any way, are unsafe for use between test fixtures, test methods, and even potentially parameterized executions of the same test method. That is, of course, unless they generate completely new instances of everything on each use or in any other reliable way can ensure that the same references are never provided to or otherwise reused by more than one instance of one test method (such as IEnumerables used as parameter sources for only single test methods, which is a safe and common practice).
There's also a potential need for the SetUp and TearDown to be re-considered and possibly made a bit more formal. This will likely affect test execution time, but validity is more important than that, for tests. Also, if things are well-isolated, parallel test execution becomes safe and can largely mitigate that impact, anyway (or possibly even make them faster than they are now).
These are just the thoughts I had while posting this issue. None, some, or all of it may be relevant, pending actually diving into the code, or there may be other work items that come up, in the process, as well.
I'm going to work on this before #29, since this class is a base dependency for literally every test, so it makes more sense to deal with this first, and then move on to derived classes.