Skip to content
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

Fix S1607: Should be applied for test classes as well #1124

Closed
mandjeo opened this issue Jan 31, 2018 · 3 comments
Closed

Fix S1607: Should be applied for test classes as well #1124

mandjeo opened this issue Jan 31, 2018 · 3 comments
Assignees
Labels
Type: False Negative Rule is NOT triggered when it should be.
Milestone

Comments

@mandjeo
Copy link

mandjeo commented Jan 31, 2018

Description

Since tests can be ignored by putting the [Ignore] attribute on the test class, I think it would make sense that the rule as also checking those cases as well.
Now it's just checking if the [Ignore] is put on the test method level.

RSPEC-1607

Repro steps

Put [Ignore] next to the [TestClass] attribute and all the tests are ignored but the rule is not triggered.

Expected behavior

Rule should be triggered for the test class as well.

Additional improvement to be considered

  • Another rule which would fail if the Ignore attribute is put on the class level would also be satisfactory.

  • In our team people are putting the comment for ignoring the test as a parameter of the Ignore attribute. Maybe the rule could also take that comment as a valid fix.
    For example:
    [Ignore("Test is ignored because....")]

@Evangelink
Copy link
Contributor

Hi @mandjeo,

I think the first part of the suggestion (checking the class level [Ignore]) is a good idea! Thanks for raising it.

Regarding the additional improvement, could you tell me where is defined the IgnoreAttribute you are using? Because the one defined in Microsoft.VisualStudio.TestTools.UnitTesting.IgnoreAttribute doesn't have the message constructor parameter.

@valhristov valhristov changed the title Rule S1607 Should be applied for test classes as well Fix S1607: Should be applied for test classes as well Jan 31, 2018
@Evangelink Evangelink added Area: Rules Type: False Negative Rule is NOT triggered when it should be. labels Jan 31, 2018
@mandjeo
Copy link
Author

mandjeo commented Jan 31, 2018

Hi @Evangelink
Thanks for the very(!) prompt reply.
Actually we are using the IgnoreAttribute from the Microsoft.VisualStudio.TestTools.UnitTesting. I also saw that in the documentation there is no constructor with the message, however you can add it in the code. And even in VS you get the IntelliSense where it says that there is as overload with the message.
If you navigate to the definition you'll also see the constructor overload and a property for the IgnoreMessage.
We are using .NET 4.6.2.

@Evangelink
Copy link
Contributor

The ctor overload with the message is actually available only when using the MSTest NuGet so I have to say +1 for the support of this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: False Negative Rule is NOT triggered when it should be.
Projects
None yet
Development

No branches or pull requests

3 participants