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

NUnit question/ request? #637

Closed
mmcedora opened this issue Jul 19, 2023 · 14 comments
Closed

NUnit question/ request? #637

mmcedora opened this issue Jul 19, 2023 · 14 comments

Comments

@mmcedora
Copy link

I am experimenting with FsCheck using C# and NUnit as shown below in the code. I wonder why properties MUST be declared with a verbose syntax as in method 'ThisWorks' while the much simpler version 'ThisShouldAlsoWorkButItWontCompile' gives a compilation related to the FsCheck Property attribute error saying: "The test method has '2' parameter(s), but only '0' argument(s) are supplied by attributes."

It should be possible to transform the latter function into the first one automatically by FsCheck. It would result in much more readable/maintainable/portable tests. Why not?

public class NUnitTests
{
    private int Add(int x, int y) // Method under test
    {
        return x + y;
    }

    [Property(Verbose = false, MaxTest = 200)]
    public Property ThisWorks()
    {
      Func<int,int,bool> summer = (a,b) => Add(a,b) == a + b;
      return Prop.ForAll(summer);
    }
    
    [Property(Verbose = false, MaxTest = 200)]
    public bool ThisShouldAlsoWorkButItWontCompile(int x, int y)
    {
        return Add(x,y) == x+y;
    }
}
@ploeh
Copy link
Member

ploeh commented Jul 19, 2023

I'm fairly certain that the xUnit.net [Property] attribute works that way - at least if you make it a void instead of a bool method, and use an assertion library to verify the outcome of the test.

Have you tried that?

@mmcedora
Copy link
Author

I have not used XUnit (we use NUnit) but I think I read somewhere that xUnit and NUnit behaves different with the Property attribute so you are properly right.

@mmcedora
Copy link
Author

In the NUnit C# example in FSCheck repo, I noticed a comment saying that what I want for a "Simple boolean Function" but it says nothing about two-argument functions like this one.

@kurtschelfthout
Copy link
Member

Property supports both cases fine: https://github.com/fscheck/FsCheck/blob/master/examples/FsCheck.NUnit.CSharpExamples/Examples.cs#L14

Where is that "compile error" coming from? csc doesn't know about FsCheck, or test methods. Is this the NUnit equivalent of #636?

@mmcedora
Copy link
Author

It apparently comes from NUnit - The error message also mentions NUnit1027 which is described here "https://docs.nunit.org/articles/nunit-analyzers/NUnit1027.html"

I have tried to disable the error using "#pragma warning disable NUnit1027" but this just give another error "No test is available in xxxxx. Make sure that test discoverer & executors are registered and platform & framework version settings are appropriate and try again"

I also tried to add an extra attribute [TestCase(0,0, ExpectedResult = true)] below [Property] which compiles but then the Property attribute is ignored. So not good either.

Maybe the implementation of the [Property] attribute defined by FsCheck should be changed to something simar to [TestCase] but with the right behavior of cause ???

@ploeh
Copy link
Member

ploeh commented Jul 20, 2023

If the issue is with an NUnit analyser, it might be a bug in the analyser. We recently had a similar issue with the xUnit.net analyser, and the xUnit.net maintainers accepted it as a bug at their end.

I'm not familiar with the NUnit implementation or extensibility model, but perhaps it might be appropriate to follow such a line of inquiry.

@mmcedora
Copy link
Author

I don't think NUnit will consider it a bug on their part - I think it is the FsCheck Property attribute that done wrong.

@ploeh
Copy link
Member

ploeh commented Jul 20, 2023

I don't think NUnit will consider it a bug on their part

Why not?

@mmcedora
Copy link
Author

Because (my Guess) from NUnits point of view it thinks of FsCheck's [Property] as similar to the [Test] attribute where this behavior is wrong. If FsCheck changes it's implementation to be similar to NUnit's [TestCase] attribute there would be no problem. Hence, they will properly say that FsCheck coded its [Property] attribute wrong.

@kurtschelfthout
Copy link
Member

FsCheck’s Property is only superficially similar to TestCase. It is not possible to do what you suggest.

The NUnit analyser is clearly too eager here. Turn it off and/or check in with the NUnit people so they can fix it.

@mmcedora
Copy link
Author

I have tried to disable the particular error in the Nunit analyser but than the (Property) test compiles but is not executed.

@ploeh
Copy link
Member

ploeh commented Jul 20, 2023

I have tried to disable the particular error in the Nunit analyser but than the (Property) test compiles but is not executed.

Yes, I can reproduce that, but if I add [TestFixture] to the class, both tests run.


When a framework like xUnit.net or NUnit exposes extensibility points, it's to be expected that third parties take advantage of these - sometime in unexpected ways. Here, FsCheck is the third party.

When the good xUnit.net people changed their analyser rule, they seem to have forgotten to take third party extensibility into account (which is understandable), but as soon as we told them, they immediately realised that they'd been 'too eager', as @kurtschelfthout put it.

It still sounds to me as though something similar is going on here.

@mmcedora
Copy link
Author

See also Issue 623: #623

@kurtschelfthout
Copy link
Member

Closing as dupe

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

No branches or pull requests

3 participants