RFC: Ignore/Skip test or test case

[Evangelink]

RFC: Ignore/Skip test or test case

- [ ] Approved in principle

• Under discussion
• Implementation
• Shipped

Summary

Improve test/test case skipping mechanism.

Motivation

Today, it is possible to use IgnoreAttribute, that can be set on a method or on a class to allow ignoring respectively a test method or all test methods of a given class.

There are multiple design issues with this solution:

1. It is possible to set it on any class or method even if they are not a test class or test method. When set in some invalid location, the attribute will have no effect so the impact on user is limited but this is providing a bad user experience. This can also be confusion for users in contexts such as initialization or cleanup methods ([AssemblyInitialize], [ClassInitialize], [TestInitialize], [AssemblyCleanup], [ClassCleanup], [TestCleanup]) where it might be thought this would ignore the marked initialize/cleanup method.

2. It is not possible to ignore only specific entries of parameterized tests. For example, it's currently impossible to ignore a DataRow entry (see How to ignore particular test case (DataRow) #1043).

Detailed design

We would like to obsolete the existing attribute and replace it with some property (e.g. Ignore = "some reason") on the various attributes where we want to support this feature.

By doing so, we ease discoverability as the property is available on the attribute we have already defined and we would fix the design issues raised above.

We would keep the multi-level support:

1. If a test class is marked as ignored (e.g. [TestClass(Ignore = "...")]) then all test methods are ignored.

2. If a data test method is marked as ignored (e.g. [DataTestMethod(Ignore = "...")]) then all test cases are ignored.

To complete furthermore this feature, we may also want to introduce some new concepts for other data sources:

1. Another property like IgnoreRows that would allow to specific a list of rows to skip with either only 1 message or a list of messages (e.g. [DynamicData(IgnoreRows = ([0, 2, 3], "message"))] or [DynamicData(IgnoreRowIndexes = [(0, "message 1"), (2, "message 2"), (3, "message 3")])]).

2. Replace (or add) the object[] part of IEnumerable<object[]> by some specific type (we could reuse DataRow or introduce some new type) that would allow to skip the entry for all tests using this source.

Drawbacks

Users would need to update their tests to use the new syntax if we obsolete the old one.

Alternatives

We could extend current [Ignore] attribute to provide properties for rows to ignore.

There aren't lot of requests/complaints about the discoverability or the fact this is not possible to suppress a specific row/entry of a parameterized test.

Compatibility

Not per se but the goal is to drop the older solution.

Unresolved questions

None.

[fforjan]

Personal comment here, I like that it is a specific attribute: when i do a code review, I do not see a modify attribute change but a attribute being remove or added which is simple to review - full line added or remote, less change of merge conflict ?
As such I dis-like the one which does the Ignore property on the test method .

So potentially for a data driven method, do you prefer one attribute for all, or one attribute per ignore ?
In the second case, you could allow multiple ignore attributes with a optional index only use for the datadriven one ?

Extra requests, could we have created our own ignore attribute ? For example, in our case, we would like the developer to give 2 informations, one being the work item to address the ignore command , the second the message so we would like to have our own attribute like [MyProjectIgnore(1234, "message")] but as the attribute is sealed, we cannot really do anything :(

[Evangelink]

Hi @fforjan, thanks for the valuable feedback!

[nohwnd]

One more drawback is that the new approach would make it different from other attributes that provide metadata or configure execution like TestCategory, Owner, Timeout.

[Evangelink]

@nohwnd Given #4089 I actually feel the exact opposite. We have more and more requests to have the attributes being associated to a specific DataRow entry and there is technically no other solution than properties.

[nohwnd]

I agree that the solution for ignoring a specific data row is best done with a property on that data row, rather than some hacky ambiguous solution with attribute placement or order.

What I don't agree with is changing all the other patterns to this, just to enable a minor scenario (in my view). Do we have some other motivations, and major scenarios that this change would enable?

I see two different reasons for this change in the linked issue:

• We don't want to comment out the Data Row because it is not allowed / we want to keep track of it in the future.
  • This seems valid, but also highlights that we would need to add way more the the attribute than just skip and skip reason. For the best experience we would also need to support all the other attributes that the user might have implemented to track their issues (e.g. GithubLink). So in this case a better support for re-factoring datarow into a seperate test that uses the same method body seems to solve this problem better and more systematically. And opens other possibilities.
• We have tests that run for long time but are still occasionally useful.
  • This is basically the definition of Explicit tests in NUnit and Xunit, implementing those into mstest would most likely serve the OP better, because they would not have to comment and uncomment their tests (or add and remove ignore attribute).

Last point that is not mentioned here, is that DataRow tests are specific implementation of ITestDataSource, and so the Ignore functionality should probably be propagated down to that level as well. Which seems difficult, given that we don't normally wrap the data produced by the producer into any wrapper that would be able to describe the ignore.

[Evangelink]

My idea would be the following:

• Keep [Ignore] attribute on class and methods
• Allow to have a Ignore property on
  • [TestClass] - equivalent to [Ignore] on the class but added for symmetry
  • [TestMethod] - equivalent to [Ignore] on the method but added for symmetry
  • [DataRow] - ignore specific entry
  • [DynamicData] - ignore source
• Add a new type TestData or TestDataRow that would be special cases under the IEnumerable<object[]> return type of ITestDataSource and would allow to fit all the special needs for a given row (e.g. Ignore, Traits, Owner, DisplayName...)

[nohwnd]

ITestDataItem ? And add the data uid?