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

MSTEST0015: Test method should not be ignored shouldn't be enabled in Recommended (maybe even All) #4780

Closed
Youssef1313 opened this issue Jan 25, 2025 · 3 comments · Fixed by #4784

Comments

@Youssef1313
Copy link
Member

No description provided.

@Evangelink
Copy link
Member

+1 for dropping from Recommended. Not sure what to do with All. In a way, it's nice to respect the semantic but it also feels like this rule is more of a "audit" kind of rule that you want to try out once in a while.

@Youssef1313 do you know what roslyn does for "all"? Is it truly all?

@Evangelink Evangelink added this to the MSTest 3.8 / Platform 1.6 milestone Jan 25, 2025
@Youssef1313
Copy link
Member Author

@Evangelink For roslyn-analyzers, "All" is not truly all. And that's the case for MSTest today as well.

For roslyn-analyzers, they have isEnabledByDefaultInAggressiveMode (which is commonly set to false for rules that are marked CandidateForRemoval, but it's not limited to CandidateForRemoval).

For us, we already disable the "conflicting" rules (Dispose vs Cleanup and ctor vs Init) in "All" mode. I named the flag as disableInAllMode as I found the name more clear than isEnabledByDefaultInAggressiveMode

@Evangelink
Copy link
Member

Yes I saw that part and conflicting rules do make sense to be disabled (or to have a side selected). I think the ignore is hitting yet another space. I feel like we should start with it disabled and monitor usages, if we see many people enabling it then we can change this decision.

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.

2 participants