fscheck / FsCheck

Random Testing for .NET
https://fscheck.github.io/FsCheck/
BSD 3-Clause "New" or "Revised" License
1.15k stars 154 forks source link

NUnit question/ request? #637

Closed mmcedora closed 11 months ago

mmcedora commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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

Why not?

mmcedora commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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

ploeh commented 11 months ago

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 commented 11 months ago

See also Issue 623: https://github.com/fscheck/FsCheck/issues/623

kurtschelfthout commented 11 months ago

Closing as dupe