Author Tests assume ICustomAttributeProvider.GetCustomAttributes return order is defined

[oznetmaster]

After some research, it appears that the order of the attributes returned by ICustomAttributeProvider.GetCustomAttributes is undefined (random).

However, it seems like the assumption is made in Nunit that custom attributes are always returned in the order they appear in the source code.

For example, the AuthorAttribute allows multiple occurrences of the attribute. The tests for multiple AuthorAttribute assumes a specific order for the values in the resulting Author element in the test's Properties. That test can fail because the order is actually not defined.

[oznetmaster]

This is why some attributes actually have an Order property, like some of those in Asp.Net.

[oznetmaster]

I have not examined all the other AllowMultiple attributes in NUnit to see if any of them have similar problems. That is why I did not make this specific to the Author attribute

[CharliePoole]

There is no such assumption in NUnit. In fact, I've made many posts warning users of this and we even have some documentation about the unreliablility of order assumptions WRT data attributes.

However, it's entirely possible that some particular test is making that assumption. One moment...

...I'm back! You're right about the Author tests. The recently added tests for multiple authors use Is.EqualTo to compare arrays but should use Is.EquivalentTo

@JustinRChou you added these tests last week, would you like to take on this issue to fix them?

@oznetmaster If you see others, please continue to report them. Too bad you didn't review @JustinRChou's PR last week!

[JustinRChou]

The Author property seemed to be well behaved in terms of the stack order. But that order is enforced by the test properties.

Never tried comparing it to customattributes.

[oznetmaster]

I only found this when I ported these changes to my CF build, and the Author test failed.

I had never specifically thought about the issue before, perhaps intuitively just not doing it :(

I did review the PR, but did not notice the potential problem.

[oznetmaster]

In my case, TestMethodMultipleAuthors returned them in the order Rob Prouse, NUnit, Charlie Poole.

TestFixtureMultipleAuthors returned them in the order specified in the test.

That is, unfortunately, the nature of undefined :)

[CharliePoole]

@JustinRChou The order of the property values is the same as the order that the attributes were found. So they will be different when using different runtimes. We sometimes see differences between Mono and .NET and always between .NET and .NETCF. Because we no longer build CF, I guess it's now easier for a mistake to slip through. In fact, IIRC, I think I suggested you use Is.EqualTo in the first place! 😊

[oznetmaster]

Changing to Is.EquivalentTo fixes the tests

[JustinRChou]

@CharliePoole I could modify the tests.

[oznetmaster]

If there is a desire to allow a specific ordering of authors, then an optional property Order could be added to the AuthorAttribute.

[CharliePoole]

@JustinRChou I assigned this to you.

@oznetmaster No reason to need ordering that I can see.