← Back to team overview

nunit-core team mailing list archive

Re: [Merge] lp:~jterrell/nunitv2/action-attributes into lp:nunitv2

 

Review: Needs Fixing
Comments...

1) After merging with the latest changes to trunk, there were a number of errors caused by having
SampleAttribute at the assembly level in test-assembly.dll. The error is due to a null reference in the _Results field.

2) The above failures led to an unhandled exception in the TestSuite code that propogates errors to 
child tests. TestSuite code has the assumption that any suite that can throw an exception or otherwise cause an error in its one time setup must have a fixture specified. With this change that's no longer the case so all that code will need to be reviewed. I have a fix for the specific instance, which I can send you.

3) I like the way you get a Type for each interface and reuse it, without needing a reference to the framework assembly. Have you verified that this works in all runtimes and versions? If so, it's a trick that we can probably use elsewhere.

4) You introduced a new parameter 'Assembly' to several methods of TestAssembly, rather than using the instance member. Any reason for this? Is it purely a matter of style?

5) The order of execution of Test Actions with respect to the various types and levels of setups needs to be discussed as a design issue. Running before actions all together after all setup doesn't seem to be the right way to do it from a usability point of view. The order of actions specified on a base class as opposed to those on a derived class looks like it's indeterminate as well.

6) I'd like to have further discussion about the general design as it looks to the user. I think this can happen in parallel with getting any code fixes done.

Actions...

Please fix item 1. If possible, let's not have an assembly level attribute on TestAssembly that impacts large numbers of unrelated tests.

I'll take a look at item 2 and also at possibly separating the code that requires a fixture into
TestFixture rather than TestSuite.

I'll post on the list about the order of running actions, setups and teardowns. We can then discuss and come up with a solution you can implement.

We'll need tests of the ordering and of what happens when an exception is thrown.

Discussion of the overall interfaces will be on the list as well.

I'm planning to merge this into a 2.6 release, which we can work on incrementally leading up to
a beta and a final release. 2.5.x will continue to get bug fixes only.

-- 
https://code.launchpad.net/~jterrell/nunitv2/action-attributes/+merge/43316
Your team NUnit Core Developers is requested to review the proposed merge of lp:~jterrell/nunitv2/action-attributes into lp:nunitv2.