NUnit failures on list-ops test

[rmunn]

While working on the list-ops exercise, I was getting some failures on a foldr implementation that I was certain was correct. The failures looked like this:

1) SetUp Error : ListOpsTest.foldr as append
   SetUp : System.NullReferenceException : Object reference not set to an instance of an object
  at NUnit.Core.NUnitFramework.GetResultState (System.Exception ex) <0x40d86ce0 + 0x0004c> in <filename unknown>:0 
  at NUnit.Core.TestMethod.RecordException (System.Exception exception, NUnit.Core.TestResult testResult, FailureSite failureSite) <0x40d86c10 + 0x00073> in <filename unknown>:0 
  at NUnit.Core.TestMethod.RunTestCase (NUnit.Core.TestResult testResult) <0x40d54130 + 0x00110> in <filename unknown>:0 
  at NUnit.Core.TestMethod.RunTest () <0x40d53640 + 0x0012f> in <filename unknown>:0 

After some troubleshooting, I discovered that my implementation was correct, and the NullReferenceException was happening inside NUnit 2.6.3 when it tried to compare two very large F# lists. I was able to prove this by creating a minimal example that fails:

module ListOpsTest
open NUnit.Framework
let big = 100000
[<Test>]
let ``torture nunit`` () =
    let l1 = [1 .. big]
    let l2 = [1 .. big]
    Assert.That(l1, Is.EqualTo(l2))

This also produces a NullReferenceException:

1) SetUp Error : ListOpsTest.torture nunit
   SetUp : System.NullReferenceException : Object reference not set to an instance of an object
  at NUnit.Core.NUnitFramework.GetResultState (System.Exception ex) <0x40fea4f0 + 0x0004c> in <filename unknown>:0 
  at NUnit.Core.TestMethod.RecordException (System.Exception exception, NUnit.Core.TestResult testResult, FailureSite failureSite) <0x40fea420 + 0x00073> in <filename unknown>:0 
  at NUnit.Core.TestMethod.RunTestCase (NUnit.Core.TestResult testResult) <0x40fd0110 + 0x00110> in <filename unknown>:0 
  at NUnit.Core.TestMethod.RunTest () <0x40fcf620 + 0x0012f> in <filename unknown>:0 

By changing the assertion to Assert.That((l1 = l2), Is.True), I was able to make my minimal test case pass rather than throw NullReferenceException during the comparison.

So I replaced the foldr tests in the list-ops exercise with the following two tests, and NUnit stopped throwing NullReferenceExceptions and started passing instead:

[<Test>]
let ``foldr as id`` () =
    let result = foldr (fun item acc -> item :: acc) [] [1 .. big]
    let expected = [1 .. big]
    Assert.That((result = expected), Is.True)

[<Test>]
let ``foldr as append`` () =
    let result = foldr (fun item acc -> item :: acc) [100 .. big] [1 .. 99]
    let expected = [1 .. big]
    Assert.That((result = expected), Is.True)

This loses NUnit's nice list-comparison features where it tells you what items were different and at what index they were found... but this is the only way I've found so far to get the tests to run correctly.

[rmunn]

I also had to tweak two more tests where NUnit was failing to compare two large lists:

[<Test>]
let ``append of large lists`` () =
    let result = append [1 .. (big / 2)] [1 + big / 2 .. big]
    let expected = [1 .. big]
    Assert.That((result = expected), Is.True)

[<Test>]
let ``concat of large list of small lists`` () =
    let result = concat (map (fun x -> [x]) [1 .. big])
    let expected = [1 .. big]
    Assert.That((result = expected), Is.True)

With those tweaks, the unit tests all passed.

This is with NUnit 2.6.3 on Mono 4.2.4 (I downgraded from Mono 4.4 to 4.2 on my main F# dev box because using Mono 4.4 was triggering fsprojects/FAKE#1196, but downgrading Mono to 4.2 fixed that issue).

[ErikSchierboom]

@rmunn That is weird. Could you perhaps try it using NUnit 3.2?

[ErikSchierboom]

@rmunn Were you able to reproduce this on NUnit 3.2?

[rmunn]

Running it with the NUnit 3.2 console runner, I got a Stack overflow in unmanaged: IP: 0x5f4bbd, fault addr: 0x7f3d184b9ff8 error when I used the Assert.That(l1, Is.EqualTo(l2)) approach. But then I noticed I was still compiling against the NUnit 2.6 DLLs, even though the .fsproj file I'm using was pointing at an NUnit 3.2 nunit.framework.dll -- instead of grabbing the DLL pointed at by the HintPath, it was grabbing the DLL from my system's GAC in /usr/lib/cli/nunit.framework-2.6.3/.

Rather than futz around for hours trying to figure out the intricacies of MSBuild XML files, I then just used Paket: I cloned a new copy of the Project Scaffold, set its paket.dependencies to request NUnit 3 insteaed of 2 (which got me NUnit 3.4.1), and copied the Assert.That([1 .. big], Is.EqualTo([1 .. big])) test into the project. NUnit 3.4.1 ran that test just fine, as did NUnit 3.2.1. But when I ran it against NUnit 2.6.3, I got the same NullReferenceException as before. The only time when I saw that "stack overflow" error was when compiling against NUnit 2.6.3 and using an NUnit 3.2 runner.

Conclusion: It's an NUnit 2 bug that makes it fail when it tries to traverse very large lists to compare them for equality, and this bug has been fixed in NUnit 3.

[ErikSchierboom]

@rmunn Thanks for checking! I'll close this issue then.

[rmunn]

Although I agree with closing the issue, I also suggest adding a comment to the unit test(s) that fail with 2.6 saying, "If you find that you get a NullReferenceException running this particular test, either run it with NUnit 3, or else comment this test out and uncomment the one below". And the one below is the same test rewritten with a Assert.That(actual, Is.EqualTo(expected)) style.

That way someone else who tries to use NUnit from their Ubuntu system's apt-get packages won't be left scratching their heads and wondering what's wrong with their code.

[ErikSchierboom]

I'm not sure about adding a commented, alternative version of the test. This to me increases confusion. Adding a comment about an exception with NUnit 2.6 is of course a good idea.

[ErikSchierboom]

@rmunn One last thing, could you try to run it in NUnit 2.6 with let big = 50000 (half as big) or let big = 10000? I am wondering where the cutoff point is.

[rmunn]

After an involuntary reboot (brief power outage), I can't actually get the unit test in the exercise to fail under 2.6 any longer! However, in my dedicated project I set up to test against NUnit 2.6 or 3.2 (using Paket and FAKE to handle the messy details for me), I was still able to reproduce the failure in 2.6. Here are my results:

big = 100,000 -- 💥
big = 10,000 -- 🆗
big = 50,000 -- 💥
big = 25,000 -- 🆗
big = 32,767 -- 🆗
big = 32,768 -- 🆗
big = 42,000 -- 🆗
big = 48,000 -- 💥
big = 45,000 -- 💥
big = 43,500 -- 💥
big = 42,500 -- 🆗
big = 43,000 -- 💥
big = 42,750 -- 💥, but in a different way. This time I got:

Errors and Failures:
1) Test Error : TestNUnit.Tests.torture nunit
   System.StackOverflowException : The requested operation caused a stack overflow.
  at <0x00000 + 0x00000> <unknown method>
  at (wrapper alloc) System.Object:AllocSmall (intptr,intptr)
  at Microsoft.FSharp.Collections.FSharpList`1[T].GetHashCode (IEqualityComparer comp) <0x41b0d390 + 0x00083> in <filename unknown>:0 
  at Microsoft.FSharp.Collections.FSharpList`1[T].GetHashCode (IEqualityComparer comp) <0x41b0d390 + 0x00057> in <filename unknown>:0 
  at Microsoft.FSharp.Collections.FSharpList`1[T].GetHashCode (IEqualityComparer comp) <0x41b0d390 + 0x00057> in <filename unknown>:0 
  at Microsoft.FSharp.Collections.FSharpList`1[T].GetHashCode (IEqualityComparer comp) <0x41b0d390 + 0x00057> in <filename unknown>:0 
... snip 993 identical lines ...


Running build failed.
Error:
NUnit test failed (1).

---------------------------------------------------------------------
Build Time Report
---------------------------------------------------------------------
Target         Duration
------         --------
Clean          00:00:00.0028852
AssemblyInfo   00:00:00.0226019
Build          00:00:06.7831392
CopyBinaries   00:00:00.0059822
Total:         00:00:09.0341117
Status:        Failure
---------------------------------------------------------------------
  1) Fake.UnitTestCommon+FailedTestsException: NUnit test failed (1).
  at Fake.NUnitSequential.NUnit (Microsoft.FSharp.Core.FSharpFunc`2 setParams, IEnumerable`1 assemblies) <0x41a690c0 + 0x00393> in <filename unknown>:0 
  at FSI_0005.Build+clo@146-11.Invoke (Microsoft.FSharp.Core.Unit _arg7) <0x41a69040 + 0x0005f> in <filename unknown>:0 
  at Fake.TargetHelper+targetFromTemplate@195[a].Invoke (Microsoft.FSharp.Core.Unit unitVar0) <0x41a59a70 + 0x00023> in <filename unknown>:0 
  at Fake.TargetHelper.runSingleTarget (Fake.TargetTemplate`1 target) <0x41a4ec20 + 0x000ca> in <filename unknown>:0 
---------------------------------------------------------------------

Total of 997 at Microsoft.FSharp.Collections.FSharpList1[T].GetHashCode ...` lines in that stacktrace.

[ErikSchierboom]

Thanks a lot for testing. It seems to me we have four options:

1. Reduce the number of items in the "big" list to a smaller value that doesn't crash NUnit (10.000 or 25.000 or something like that).
2. Add a comment to each of the exercises that might fail suggesting what to do.
3. Add one comment at the top of the tests code explaning that some tests might fail and what to do.
4. Rewrite the assertions to use the format that doesn't crash (using the = operator).

I don't really like the comment options, as I think that there's a fair chance people might not actually read the test files that closely. My preference is thus for option 1 or 4.

[ErikSchierboom]

@jwood803 @kytrinyx What do you think about this issue?

[jwood803]

I wonder how much effort would there be to do 4...

[ErikSchierboom]

Not much, I'll do this next week.

[rmunn]

The one drawback to 4 is that if you get something wrong so the unit test fails, you don't get to see what index compared unequal and the values at that index, just that the expected and actual lists weren't equal. But that can easily be solved by a debugger, or by the "poor man's debugging" method of just sticking printfn statements all over. So I think that 4 is probably the best way to approach this.

[kytrinyx]

Rewrite the assertions to use the format that doesn't crash (using the = operator).

I do like the idea of having something that doesn't crash.