Skip to content

Support multiple Author attributes per test #1000

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
DerekSMorin opened this issue Nov 6, 2015 · 11 comments · Fixed by #1977
Closed

Support multiple Author attributes per test #1000

DerekSMorin opened this issue Nov 6, 2015 · 11 comments · Fixed by #1977

Comments

@DerekSMorin
Copy link

Support multiple Author attributes per test

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

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

@CharliePoole CharliePoole modified the milestone: Ideas Dec 5, 2015
@CharliePoole CharliePoole modified the milestone: Ideas Jun 24, 2016
@CharliePoole
Copy link
Member

@nunit/contributors Here's another that has been waiting for comments for a year. Let's clean up!

@rprouse
Copy link
Member

rprouse commented Nov 20, 2016

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.

@immeraufdemhund
Copy link

I don't like the idea of having an AuthorAttribute as I've subscribed to the notion that all code you touch becomes your responsibility to a certain degree. Another reason I don't like it is because of source control. If I REALLLY have to find out who was the first person to write this I can use history. Lastly if it really is that important to know who the author(s) is than why isn't there an automated header on each file that was inserted on creation that you could scroll up to and read/modify. My vote is strongly against having any AuthorAttribute.

@CharliePoole
Copy link
Member

Speaking as a coach, I agree with you. I would not like to see any of my
teams use this.

Speaking as a tool developer, i have to take a different view. Some folks
asked for the attribute and we agreed to provide it. Different teams work
in different ways.

The attribute has existed for a number of releases and I don't see removing
it as an option. This issue is about allowing it to appear multiple times.
I'm in favor because restricting it feels arbitrary.

Thanks for your comment. It has given me an idea about writing something
from my personal point of view about how I think NUnit should be used.

On Nov 20, 2016 4:27 AM, "Robert Snyder" notifications@github.com wrote:

I don't like the idea of having an AuthorAttribute as I've subscribed to
the notion that all code you touch becomes your responsibility to a certain
degree. Another reason I don't like it is because of source control. If I
REALLLY have to find out who was the first person to write this I can
use history. Lastly if it really is that important to know who the
author(s) is than why isn't there an automated header on each file that was
inserted on creation that you could scroll up to and read/modify. My vote
is strongly against having any AuthorAttribute.


You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
#1000 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACjfhZAY8vGHigzrSzK5Kx-92CpG2g18ks5rADyogaJpZM4Gdqa3
.

@CharliePoole
Copy link
Member

@rprouse I'd say allow it. Again, we don't use the attribute, so it should be no more than changing a property on the attribute usage.

@JustinRChou
Copy link
Contributor

Is this still relevant?

So the recommended way of adding authors is using the AuthorAttribute?
The Test author property can be ignored?

@CharliePoole
Copy link
Member

@JustinRChou Yes, it has been marked as an enhancement and added to the Backlog. Nobody is working on it. You can take it if you want it, therefore. 😄

The Author property on tests is set by adding the AuthorAttribute to the test method. Currently, it is only allowed to appear once. If you change it to allow multiple Author attributes the only other thing needed is to see how the property is used. PropertyBag allows adding multiple values with the same key but only if you use the right method to do it. See Set versus Add methods.

@JustinRChou
Copy link
Contributor

JustinRChou commented Jan 10, 2017

I was referring to the following usage.

[Test(Author = "Rob Prouse")]

I don't see how would it work unless you use a delimiter, or add a Authors property.

@CharliePoole
Copy link
Member

I forgot we did that! We could make the two work together or we could deprecate the property. Alternatively we could ignore or overwrite one of them.

Since it's low priority, we shouldn't get too complicated.

@JustinRChou
Copy link
Contributor

I would be fine with fixing the AuthorAttribute and updating the documentation to recommend using it.

@JustinRChou JustinRChou self-assigned this Jan 10, 2017
@JustinRChou
Copy link
Contributor

I just noticed that the TestCaseSource doesn't have the Author property?

And just making a note here.
I don't think it would be common if someone wants to author a TestCase like so
[TestCase(5, Author = "Charlie Poole"), Author("Rob Prouse"), Author("NUnit")]
which would only apply "Charlie Poole" to the TestCase while "Rob Prouse" and "NUnit" are applied to the TestMethod.

JustinRChou added a commit to JustinRChou/nunit that referenced this issue Jan 12, 2017
Update to Author Attribute to allow for multiple use.
JustinRChou added a commit to JustinRChou/nunit that referenced this issue Jan 12, 2017
Just a minor fix to the ordering of the assert.
JustinRChou added a commit that referenced this issue Jan 13, 2017
* #1000 Multiple Author Attribute

Update to Author Attribute to allow for multiple use.
@CharliePoole CharliePoole added this to the 3.7 milestone Jan 13, 2017
JustinRChou added a commit to JustinRChou/nunit that referenced this issue Jan 13, 2017
* nunit#1000 Multiple Author Attribute

Update to Author Attribute to allow for multiple use.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants