Should not report tests in abstract class as invalid

[Kenneth Xu]

Making a class abstract should be sufficient to indicate the test fixture is intended to be inherited by other concrete test fixture.

The problem occurs when the said abstract test fixture inherits from a 3rd party test fixture that already has TestFixture attribute, there is no way to avoid the warning now.

See http://groups.google.com/group/nunit-discuss/browse_thread/thread/b0d491a5df2e7897/3150383c3129273b

[Adam Connelly (adam-rpconnelly)]

Charlie, is this definitely a problem on the current trunk? I tried to reproduce it (by making the same changes that you made to nunitv2) and the test runner didn't produce any warnings. Also, you removed two tests about not being able to run abstract fixtures and added two tests about being able to run fixtures derived from abstract fixtures (more or less). I couldn't see anything about warnings. My understanding of this is that you still shouldn't be able to run abstract fixtures, but you shouldn't get any warnings about abstract classes that are marked as fixtures. Am I missing something?

[Charlie Poole (charlie.poole)]

What the bug reporter means is that an abstract test marked by a fixture is not runnable and shows up as ignored with a yellow bar in the gui. The tests I removed are still in the nunit-3.0 trunk, so it's still doing ignoring abstract fixtures.

To accomplish this, NUnit has to pay ignore abstract fixtures (see NUnitTestFixtureBuilder.CanBuildFrom and you have to change some code in NUnitTestFixtureBuilder.BuildFrom to make the derived class work.

Since some of the same code impacts bug 487878, you may want to look at that one at the same time.

[Adam Connelly (adam-rpconnelly)]

I think I understand what you mean now. Is the difference that if the class is abstract but not sealed, you assume that the intention is that it will be inherited from as a base test fixture, whereas if it is abstract and sealed you show a warning because it can't be a test fixture?

[Adam Connelly (adam-rpconnelly)]

I'm still confused :(. The problem I'm having is that I can add the new tests that you added, but not remove the tests that you removed. Then when I run the tests, they pass regardless of whether I make the changes to NUnitTestFixtureBuilder that you made or not. This makes me think that either the changes I'm making aren't tested properly, or they aren't actually changing the behaviour. I've pushed the changes I've made so far to https://code.launchpad.net/~adam-rpconnelly/nunit-3.0/bug-488002. Any chance you could take a look at them and give me a hand?

[Charlie Poole (charlie.poole)]

Regarding "abstract but not sealed" - that's just a convention that identifies a .NET 2.0 or later
static class. In earlier versions of NUnit, static classes could not be tests, which ruled out F#.

I'll look at the changes and get back to you.

[Charlie Poole (charlie.poole)]

This was a puzzler! But here's the answer...

1. The two tests were removed only because an abstract class
is technically neither runnable nor non-runnable. It's just
ignored by NUnit in normal operation. However, leaving the
tests in to prevent a regression is probably a good idea.

2. What was actually being fixed was not tested - or more
exactly, I did an adhoc test and then removed it. However,
I figured out a way to test it. If you add this to the
TestFixtureTests class, it will cause an error without
your changes (because it tries to build the test) but
simply ignore the class with your changes.

 [TestFixture]
 public abstract class NUnitShouldIgnoreThisAbstractTestFixture
 {
 }

Do you want me to go ahead and merge in your changes, together
with the extra test?

Charlie

PS: Thanks for all the work you have been doing. I hope you'll
be able to find time to keep it up. Let me know when you're ready
for more suggestions... :-)

> -----Original Message-----
> From: <email address hidden> [mailto:<email address hidden>] On
> Behalf Of Adam Connelly
> Sent: Sunday, December 27, 2009 9:44 AM
> To: <email address hidden>
> Subject: [Bug 488002] Re: Should not report tests in abstract
> class as invalid
>
> I'm still confused :(. The problem I'm having is that I can
> add the new tests that you added, but not remove the tests
> that you removed. Then when I run the tests, they pass
> regardless of whether I make the changes to
> NUnitTestFixtureBuilder that you made or not. This makes me
> think that either the changes I'm making aren't tested
> properly, or they aren't actually changing the behaviour.
> I've pushed the changes I've made so far to
> https://code.launchpad.net/~adam-rpconnelly/nunit-3.0/bug-4880
> 02. Any chance you could take a look at them and give me a hand?
>
> --
> Should not report tests in abstract class as invalid
> https://bugs.launchpad.net/bugs/488002
> You received this bug notification because you are a bug assignee.
>

[Charlie Poole (charlie.poole)]

I needed this in support of some work of my own, so I went ahead and merged it.