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

Exceptions are reported inconsistently #317

Closed
altxt opened this issue Feb 20, 2021 · 8 comments · Fixed by #318
Closed

Exceptions are reported inconsistently #317

altxt opened this issue Feb 20, 2021 · 8 comments · Fixed by #318
Milestone

Comments

@altxt
Copy link
Contributor

altxt commented Feb 20, 2021

It looks like exceptions in Property and Property<bool> are treated and reported differently, with each treatment having good and bad sides. The important aspects in question are

  • whether an exception in a single test run aborts the entire property test
  • whether the correct exception location is reported in the output

Property:

protected void Run(Action action) => action();

[Test]
public void HH_ExceptionInPropertyVoid()
{
    var property =
        from x in Property.ForAll(Gen.Int32(Range.Constant(0, 0)))
        select Run(() =>
        {
            _ = 1 / x != 2; // line 21
        });
    property.Check(); // line 23
}

Output:

System.Exception : *** Failed! Falsifiable(after 1 test) :
0
This failure can be reproduced by running:
> Property.recheck(1 : Size) ({ Value = 6998132399885834250UL; Gamma = 6257968130275417025UL }) <property>

at Microsoft.FSharp.Core.Operators.Raise[T](Exception exn)
at Hedgehog.ReportModule.tryRaise(Report report) in c:\test\fsharp-hedgehog\src\Hedgehog\Report.fs:line 107
at Hedgehog.Property.check(Property`1 p) in c:\test\fsharp-hedgehog\src\Hedgehog\Property.fs:line 185
at Hedgehog.Linq.PropertyExtensions.Check(Property property) in c:\test\fsharp-hedgehog\src\Hedgehog\Linq\Property.fs:line 90
at Test.HedgehogBased.HedgehogSimpleTests.HH_ExceptionInPropertyVoid() in c:\test\HedgehogBased\HedgehogSimpleTests.cs:line 23

Notice that

  • Test run and reproduction info (courtesy of Hedgehog) is present
  • Stack trace only mentions line 23, in which the property testing is initiated, but not line 21, in which an exception is actually thrown

Property<bool>:

[Test]
public void HH_ExceptionInPropertyBool()
{
    var property =
        from x in Property.ForAll(Gen.Int32(Range.Constant(0, 0)))
        select 1 / x != 2; // line 41
    property.Check();
}

Output:

System.DivideByZeroException :
at Test.HedgehogSimpleTests.<>c.<HH_ExceptionInPropertyBool>b__1_0(Int32 x) in c:\test\HedgehogBased\HedgehogSimpleTests.cs:line 41
at <StartupCode$Hedgehog>[email protected](T arg00) in c:\test\fsharp-hedgehog\src\Hedgehog\Linq\Property.fs:line 147
at [email protected](a x) in c:\test\fsharp-hedgehog\src\Hedgehog\Outcome.fs:line 28
at Hedgehog.Outcome.cata[a, b](Outcome`1 outcome, FSharpFunc`2 failure, FSharpFunc`2 discard, FSharpFunc`2 success) in c:\test\fsharp-hedgehog\src\Hedgehog\Outcome.fs:line 21
at Hedgehog.Outcome.Map[a, b] (FSharpFunc`2 f, Outcome`1 result) in c:\test\fsharp-hedgehog\src\Hedgehog\Outcome.fs:line 25
at [email protected](FSharpFunc`2 f, Outcome`1 result) in c:\test\fsharp-hedgehog\src\Hedgehog\Property.fs:line 67
at Hedgehog.Tuple.mapSnd[b, c, a] (FSharpFunc`2 f, a x, b y) in c:\test\fsharp-hedgehog\src\Hedgehog\Tuple.fs:line 12
at [email protected](Tuple`2 tupledArg) in c:\test\fsharp-hedgehog\src\Hedgehog\GenTuple.fs:line 12
at Hedgehog.Tree.map[a, b] (FSharpFunc`2 f, Tree`1 _arg1) in c:\test\fsharp-hedgehog\src\Hedgehog\Tree.fs:line 39
at [email protected](Tree`1 arg10@) in c:\test\fsharp-hedgehog\src\Hedgehog\Gen.fs:line 57
at [email protected](Seed seed, Int32 size) in c:\test\fsharp-hedgehog\src\Hedgehog\Random.fs:line 40
at Hedgehog.Random.unsafeRun[a](Seed seed, Int32 size, Random`1 _arg1) in c:\test\fsharp-hedgehog\src\Hedgehog\Random.fs:line 12
at Hedgehog.Random.run[a] (Seed seed, Int32 size, Random`1 r) in c:\test\fsharp-hedgehog\src\Hedgehog\Random.fs:line 15
at [email protected](Seed seed, Random`1 random) in c:\test\fsharp-hedgehog\src\Hedgehog\Gen.fs:line 39
at [email protected](Seed seed0, Int32 size) in c:\test\fsharp-hedgehog\src\Hedgehog\Gen.fs:line 41
at Hedgehog.Random.unsafeRun[a](Seed seed, Int32 size, Random`1 _arg1) in c:\test\fsharp-hedgehog\src\Hedgehog\Random.fs:line 12
at Hedgehog.Random.run[a] (Seed seed, Int32 size, Random`1 r) in c:\test\fsharp-hedgehog\src\Hedgehog\Random.fs:line 15
at [email protected](Seed seed, Int32 size, Int32 tests, Int32 discards) in c:\test\fsharp-hedgehog\src\Hedgehog\Property.fs:line 153
at Hedgehog.Property.reportWith'(Boolean renderRecheck, Int32 size0, Seed seed, PropertyConfig config, Property`1 p) in c:\test\fsharp-hedgehog\src\Hedgehog\Property.fs:line 165
at Hedgehog.Property.reportWith(PropertyConfig config, Property`1 p) in c:\test\fsharp-hedgehog\src\Hedgehog\Property.fs:line 169
at Hedgehog.Property.report(Property`1 p) in c:\test\fsharp-hedgehog\src\Hedgehog\Property.fs:line 172
at Hedgehog.Property.check(Property`1 p) in c:\test\fsharp-hedgehog\src\Hedgehog\Property.fs:line 185
at Hedgehog.Property.checkBool(Property`1 g) in c:\test\fsharp-hedgehog\src\Hedgehog\Property.fs:line 189
at Hedgehog.Linq.PropertyExtensions.Check(Property`1 property) in c:\test\fsharp-hedgehog\src\Hedgehog\Linq\Property.fs:line 99
at Test.HedgehogBased.HedgehogSimpleTests.HH_ExceptionInPropertyBool() in c:\test\HedgehogBased\HedgehogSimpleTests.cs:line 42

Notice that

  • There is no Hedgehog header, output starts directly with the exception info
  • Stack trace correctly starts with line 41 which actually throws the exception
@TysonMN
Copy link
Member

TysonMN commented Feb 20, 2021

Can you share these tests in a GitHub repo in which we can immediately execute and reproduce the behavior you describe?

@altxt
Copy link
Contributor Author

altxt commented Feb 20, 2021

@TysonMN
Copy link
Member

TysonMN commented Feb 22, 2021

Excellent job @altxt providing tests that minimally reproduce this issue. I just created PR #318 that fixes it.

I didn't end up needing that GitHub repo. Thank anyway though for creating it.

@ghost ghost added this to the 0.11.0 milestone Feb 22, 2021
@altxt
Copy link
Contributor Author

altxt commented Feb 24, 2021

@TysonMN thank you for the prompt response and fix.
I tried to check the fixes from your PR branch, and it looks like the issue with an exception aborting the property run is indeed fixed.
The other issue with the original exception info not being shown has propagated from Property into Property<bool> as well. Is that intentional? I'm willing to provide more context if the problem description is unclear.

@TysonMN
Copy link
Member

TysonMN commented Feb 24, 2021

The other issue with the original exception info not being shown has propagated from Property into Property<bool> as well. Is that intentional? I'm willing to provide more context if the problem description is unclear.

Oh, that is not intentional. I had read to quickly before and didn't realize there were two issues. I will check on that shortly.

@TysonMN
Copy link
Member

TysonMN commented Feb 24, 2021

I pushed new commits. I think they fix the second issue you found, which is not including the information about the exception in the report. It would be great if you could check again.

@altxt
Copy link
Contributor Author

altxt commented Feb 24, 2021

They do, thank you.
Here are the tests to prove it - not sure if I should try to add them to your branch

class MarkerException : Exception
{
    public MarkerException(string message) : base(message)
    {
    }
}

[Fact]
public void ExceptionInSelect_Action_IncludesOriginalClassMessageAndLocation()
{
    void markerThrowingFunc() => throw new MarkerException("marker message");
    var ex = Assert.ThrowsAny<Exception>(() =>
    {
        var property =
            from _ in Property.ForAll(Gen.Int32(Range.Constant(0, 0)))
            select markerThrowingFunc();
        property.Check();
    });
    var exString = ex.ToString();
    Assert.Contains("marker message", exString);
    Assert.Contains(nameof(markerThrowingFunc), exString);
    Assert.Contains(nameof(MarkerException), exString);
}

[Fact]
public void ExceptionInSelect_Func_IncludesOriginalClassMessageAndLocation()
{
    bool markerThrowingFunc() => throw new MarkerException("marker message");
    var ex = Assert.ThrowsAny<Exception>(() =>
    {
        var property =
            from _ in Property.ForAll(Gen.Int32(Range.Constant(0, 0)))
            select markerThrowingFunc();
        property.Check();
    });
    var exString = ex.ToString();
    Assert.Contains("marker message", exString);
    Assert.Contains(nameof(markerThrowingFunc), exString);
    Assert.Contains(nameof(MarkerException), exString);
}

@TysonMN
Copy link
Member

TysonMN commented Feb 24, 2021

Yes, good idea. I also thought of that last earlier today.

I just pushed more commits that make a similar assertion in the tests.

TysonMN added a commit to TysonMN/fsharp-hedgehog that referenced this issue Aug 25, 2021
TysonMN added a commit to TysonMN/fsharp-hedgehog that referenced this issue Aug 25, 2021
@ghost ghost closed this as completed in #318 Sep 4, 2021
ghost pushed a commit that referenced this issue Sep 4, 2021
TysonMN added a commit to TysonMN/fsharp-hedgehog that referenced this issue Sep 12, 2021
ghost pushed a commit that referenced this issue Sep 12, 2021
This issue was closed.
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 a pull request may close this issue.

2 participants