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

Cleanup NUnit1030 #1086

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

baranyaimate
Copy link

@baranyaimate baranyaimate commented Oct 28, 2024

Fixes the NUnit1030 warnings.

More details are available in Issue #1024.

Copy link

codecov bot commented Oct 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.22%. Comparing base (03a160c) to head (5ec8345).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1086   +/-   ##
=======================================
  Coverage   93.22%   93.22%           
=======================================
  Files         112      112           
  Lines        3410     3410           
  Branches      962      962           
=======================================
  Hits         3179     3179           
  Misses        215      215           
  Partials       16       16           
Flag Coverage Δ
?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@baranyaimate baranyaimate marked this pull request as ready for review October 28, 2024 19:04
Copy link
Member

@atifaziz atifaziz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Unfortunately, this causes a problem with test case identification because all cases appear to be named the same. Visual Studio's Test Explorer gets confused by this so instead of 700 cases appearing under NullArgumentTest like so:

image

the list gets collapsed to just 2:

image

If an individual case fails, you don't get to know which one:

image

Even the stack trace doesn't help to provide an indication:

Failed NotNull(System.Action) [< 1 ms]
Error Message:
    No exception was thrown when ArgumentNullException was expected.
Assert.That(e, Is.Not.Null)
Expected: not null
But was:  null

Stack Trace:
    at MoreLinq.Test.NullArgumentTest.<>c__DisplayClass2_0.<GetNotNullTestCases>b__1() in A:\MoreLINQ\main\MoreLinq.Test\NullArgumentTest.cs:line 54
at MoreLinq.Test.NullArgumentTest.NotNull(Action testCase) in A:\MoreLINQ\main\MoreLinq.Test\NullArgumentTest.cs:line 34
at InvokeStub_NullArgumentTest.NotNull(Object, Span`1)

Copy link
Member

@atifaziz atifaziz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@baranyaimate I looked into this quickly and it seems like that the only change that's needed is to remove the following two changes to severity:

# NUnit1030: The type of parameter provided by the TestCaseSource does not match the type of the parameter in the Test method
dotnet_diagnostic.NUnit1030.severity = suggestion
# Nunit1029: The number of parameters provided by the TestCaseSource does not match the number of parameters in the Test method
dotnet_diagnostic.NUnit1029.severity = suggestion

and then simply replace ITestCaseData with TestCaseData in all flagged cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants