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.