Support multiple TestOf attributes per test

[DerekSMorin]

Support multiple TestOn attributes per test

https://github.com/nunit/nunit/wiki/TestOf-Attribute

Note: you can currently only have one TestOf attribute per fixture or test.

[CharliePoole]

Can you explain why this would be desirable? Should we be encouraging people to write tests that do more than one thing?

[DerekSMorin]

Maybe not encouraging - but allowing for differences in the way people work. Some people have tests that encompass larger functional areas.
I'm converting some old MBUnit tests to NUnit.
Date: Fri, 6 Nov 2015 11:19:53 -0800
From: notifications@github.com
To: nunit@noreply.github.com
CC: derek_penguin@hotmail.com
Subject: Re: [nunit] Support multiple TestOf attributes per test (#999)

Can you explain why this would be desirable? Should we be encouraging people to write tests that do more than one thing?

—
Reply to this email directly or view it on GitHub.

[CharliePoole]

Fair enough. We'll see what folks think.

[CharliePoole]

@nunit/contributors I flagged this as an idea over a year ago with the comment "Let's see what people think." It has been just sitting ever since. Can we either add it to the backlog or reject it? Do you think we should do this?

[rprouse]

Like #100, I don't have a strong opinion on this. I can see the use and I don't see the implementation being much different than allowing multiple categories on a test.

[CharliePoole]

I think it only requires changing the attribute usage, since it's not an attribute NUnit uses itself. I would vote for it.

[immeraufdemhund]

I have had a few scenarios where I think this would be useful, but probably more work than a person should put into it. Such as if I was writing acceptance tests or integration tests that would use a few different classes for a desired outcome than I would possibly want to show that this adds to that code coverage. Where that woudl bite you in the butt is if you use DI and inject in different classes (such as when you add a class to care for functionality) then i'd have to update my test to reflect that this is included.

Sooo.. yes it would be nice, but I personally would want it to be a little bit smarter if it was to be used such has scanning the class for inheritance and implementations.. I will say that a counter argument to what I just said would be to make my change and run all tests and see what broke (hopefully something!).

[stevenaw]

I notice that we also use TestOf as a public property of type Type on TestAttribute and TestFixtureAttribute. Should these properties also be updated for consistency too?

If so, I can see this becoming a breaking change.
I might be missing something too. Could I ask for some guidance on an approach?

[CharliePoole]

I don't think we want to go beyond allowing multiple attributes.

[julealgon]

Was this just ignored? I see that the attribute is still AllowMultiple = false.

I was honestly surprised by the behavior when reading the documentation on this attribute. As someone pointed out, it sounds to me we should leave the decision to the user: there is just no reason to force users to comply with the "1 unit test per real class" approach.

Not allowing multiple of these attributes to me is equivalent to removing the out modifier in the language: it's usually a bad practice, but there are uses for it.

[jnm2]

I tend to agree. A unit can comprise multiple classes. Classes are the imperfect medium in which units are expressed.

Was this just ignored?

We typically rely on community members like you to help us gauge what we should put our few hours of time towards. In this instance we've agreed to the proposal and labeled it help-wanted, which means we're waiting for a motivated individual to offer a PR. Can I interest you in that? 😃

[julealgon]

We typically rely on community members like you to help us gauge what we should put our few hours of time towards. In this instance we've agreed to the proposal and labeled it help-wanted, which means we're waiting for a motivated individual to offer a PR. Can I interest you in that? 😃

That's a fair point. I didn't spot the help-wanted label, so I was just wondering if folks had agree to not do it and was thus puzzled about the reasoning.

I'll see if I can contribute a bit here, since we are relying on NUnit quite extensively in some projects. Thanks for clarifying @jnm2 .

[jnm2]

Glad to hear it! If you let us know as soon as you're ready to take it, we can remove the help wanted label. (GitHub suggests issues with this label to newcomers.)

[CharliePoole]

Aside from merely changing the attribute definition, you should examine how we access the property it creates. Our PropertyBag has differing ways of accessing properties that may appear multiple times so it could be that simply allowing multiple attributes will not automatically cause multiple properties to appear on the test. [This will make more sense when you look at the code of the attribute.]

[sunithapatel]

Hi there!

I am a beginner in contributing to open source. This issue seemed simple enough to do based on the discussion here, so I tried some changes and made a pull request for it here.

This is my first pull request! I hope I followed the process correctly. Please let me know if there is anything else I need to do.

One thing I notice that would need to be done if these changes go through is the documentation would need to be updated as well. I am guessing that would be a separate pull request as it is a separate repository?

[jnm2]

@sunithapatel Fantastic, thanks for jumping in!

The GitHub wikis don't allow pull requests, sadly, so the procedure there is for a contributor like yourself to go ahead and edit the wiki and then post a link back here for review.

[sunithapatel]

@jnm2 Sorry, newbie question, but I am not sure what is the correct way to edit the wiki.

Looking at this link: https://help.github.com/articles/adding-and-editing-wiki-pages-locally/
it says to clone the wiki locally, make changes in a branch and push that for review since only master branch is publicly visible. Is that what I need to do and then post the link to it here so you can review it?

[CharliePoole]

Our process is that we give people write access to the wiki, they change it online and we look at what they did. It relies on trust, obviously, but also on the fact that we can roll back changes. 😄

However, if you change the way TestOfAttribute works, by allowing multiple copies, we probably don't want to update the documentation till the change is actually released.

[sunithapatel]

Oh okay, now I understand. That makes sense that the documentation should not be changed till the change is released.

If you decide to include this change in a release, you can let me know then and I can help update the documentation (already have some changes locally).

Thank you again for all your quick replies and bearing with me as I understand the processes.

[jnm2]

I forgot, is it only members who can edit or can anyone edit the wiki?

Something I've done to avoid losing mental context (3.12 will probably be six months from now) is to say, "Starting in version 3.12," in the documentation and document it immediately.

[CharliePoole]

We used to have a Contributors team, which, among other things, was allowed to edit the wiki. We gave people that status at the drop of a hat. That's no longer the practice, I understand.

In V2 the docs were littered with comments about what version changes started in. With 3.0, we decided not to do that any longer but to always have the docs show the latest release. If that's still the policy, then "Starting in version 3.12" would need to be removed eventually.

I have sometimes created a new page with "New" prefixed to it for the same reason, but then you still have to go back and move it or copy the new text. It's just faster. Wish there was some automatic way to do this but lacking that I suppose we should use the approach that gives the best information in the case where we forget to go in and update it.

[jnm2]

In the back of my mind I'm hoping we find a way to host our docs on nunit.org so that contributors can offer documentation PRs.

[mikkelbu]

Merged. Thanks for the contribution @sunithapatel

[mikkelbu]

In V2 the docs were littered with comments about what version changes started in. With 3.0, we decided not to do that any longer but to always have the docs show the latest release. If that's still the policy, then "Starting in version 3.12" would need to be removed eventually.

Actually, I just stumbled upon nunit/docs#91 which proposes the opposite. Personally, I think it is fine to note when different features was introduced, but it will be of less value the older the change, so I have no problem with removing the older "tags".

[CharliePoole]

Correct. We did X then we did Y then I proposed Z in an issue that never got resolved. I'd be happy to see us document changed behavior in recent releases only.i

[CharliePoole]

Correct. We did X then we did Y then I proposed Z in an issue that never got resolved. I'd be happy to see us document changed behavior in recent releases only.i