[WIP] Investigate tests that may be intermittently failing on master. DO NOT MERGE

[blairconrad]

Prerequisites

• I have written a descriptive pull-request title
• I have verified that there are no overlapping pull-requests open
• I have verified that I am following the Nancy code style guidelines
• I have provided test coverage for my change (where applicable)

Description

[Yantrio]

what are you trying, and why are you doing it here?

[jchannon]

#2634

[blairconrad]

Thanks, @jchannon. @Yantrio, I should've said. Truth be told, I half expected it to take less time.

[blairconrad]

@jchannon, @thecodejunkie no luck in 20 tries on AppVeyor, but some success on the Travis build:

• https://travis-ci.org/NancyFx/Nancy/builds/199617620#L2653
• https://travis-ci.org/NancyFx/Nancy/builds/199585877#L2653
• https://travis-ci.org/NancyFx/Nancy/builds/199712923#L2653

Do we still consider this to not be a problem on master?

[thecodejunkie]

It's very weird since I've never seen it as part of CI failures. We've identified a couple of failing tests, on Travis, which have all been caused by Mono. To be honest I would not rule Mono out this time either since it's currently running on 4.8.0 (Stable 4.8.0.483/ba7f169 Tue Feb 7 11:47:25 UTC 2017) i.e a new version that came out yesterday. On Travis we are using latest Mono https://github.com/NancyFx/Nancy/blob/master/.travis.yml#L2 because prior versions all had their bugs and @akoeplinger recommended we move to 4.8 since that is just around the corner.

AppVeyor uses .NET Framework

[jchannon]

Are they only failing in full fwk targets? We also build for netcore on both Travis and Appveyor

…

On Wed, 8 Feb 2017 at 21:48, Andreas Håkansson ***@***.***> wrote: It's *very* weird since I've never seen it as part of CI failures. We've identified a couple of failing tests, on Travis, which have all been caused by Mono. To be honest I would not rule Mono out this time either since it's currently running on 4.8.0 (Stable 4.8.0.483/ba7f169 Tue Feb 7 11:47:25 UTC 2017) i.e a new version that came out yesterday. On Travis we are using latest Mono https://github.com/NancyFx/Nancy/blob/master/.travis.yml#L2 because prior versions all had their bugs and @akoeplinger <https://github.com/akoeplinger> recommended we move to 4.8 since that is just around the corner. AppVeyor uses .NET Framework — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2702 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGapmyhPVZyk0qKzVE4fsGFMx2AiRyWks5rajgggaJpZM4L6UDE> .

[blairconrad]

@thecodejunkie says

I would not rule Mono out this time either

@jchannon says

Are they only failing in full fwk targets? We also build for netcore on both Travis and Appveyor

I was running the tests on my home and work machines on Windows, and I see the failures .NET Framework 4.5.2.

On master, we build the production code for .NET Core, but not the tests. As I understand things, #2634 is all about starting to build (and run) the tests on .NET Core.

[blairconrad]

Have either of you tried running dotnet test -c Release a few (dozen) times in Nancy.Tests and found that all the tests pass all the time?

[thecodejunkie]

Nope.. not conservative.. I've only seen some tests fail on Travis/Mono and those were all reproducible if you ran the tests in a loop

[blairconrad]

Okay, just going out on a limb here, I'm suspicious of the static case sensitivity setting. Remember this pair of tests that failed from my rambling account of the tests I got to fail at work last night?

D:\sandbox\Nancy\test\nancy.tests [master +0 ~19 -0]> dotnet test -c Release
Project Nancy (.NETFramework,Version=v4.5.2) was previously compiled. Skipping compilation.
Project Nancy.Authentication.Forms (.NETFramework,Version=v4.5.2) was previously compiled. Skipping compilation.
Project Nancy.Testing (.NETFramework,Version=v4.5.2) was previously compiled. Skipping compilation.
Project nancy.tests (.NETFramework,Version=v4.5.2) was previously compiled. Skipping compilation.
xUnit.net .NET CLI test runner (64-bit Desktop .NET win7-x64)
  Discovering: nancy.tests
  Discovered:  nancy.tests
  Starting:    nancy.tests
    Nancy.Tests.Unit.RequestFixture.Should_respect_case_insensitivity_when_extracting_form_data_from_body_when_content_type_is_multipart_form_data [FAIL]
      Assert.Equal() Failure
      Expected: value,VALUE
      Actual:   value
      Stack Trace:
        Unit\RequestFixture.cs(330,0): at Nancy.Tests.Unit.RequestFixture.Should_respect_case_insensitivity_when_extracting_form_data_from_body_when_content_type_is_multipart_form_data()
    Nancy.Tests.Unit.Routing.DefaultRouteResolverFixture.Should_resolve_optional_capture_with_optional_specified(path: "/MOO/HOO/MOO", caseSensitive: False, expected: "HOO") [FAIL]
      Assert.Equal() Failure
      Expected: OptionalCapture HOO
      Actual:
      Stack Trace:
        Unit\Routing\DefaultRouteResolverFixture.cs(106,0): at Nancy.Tests.Unit.Routing.DefaultRouteResolverFixture.<Should_resolve_optional_capture_with_optional_specified>d__5.MoveNext()
        --- End of stack trace from previous location where exception was thrown ---
           at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
           at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
        --- End of stack trace from previous location where exception was thrown ---
           at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
           at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
        --- End of stack trace from previous location where exception was thrown ---
           at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
           at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
  Finished:    nancy.tests
=== TEST EXECUTION SUMMARY ===
   nancy.tests  Total: 1825, Errors: 0, Failed: 2, Skipped: 0, Time: 7.156s

Both Nancy.Tests.Unit.RequestFixture and Nancy.Tests.Unit.Routing.DefaultRouteResolverFixture
setStaticConfiguration.CaseSensitive`. Seems like the sort of thing that would mess us up.

As @jchannon commented (but I couldn't tell if this was a general comment aimed at all the test-running), disabling parallel testing would probably clear things up.

And as @xt0rted mentioned

After looking through the commit history some more parallel tests were disabled in #2259 and then enabled in #2490

the changes in #2490 did re-enable parallel test runs for Nancy.Tests.

That's it for now. Bedtime.

[blairconrad]

I'm conducting an experiment, repeatedly running the tests both with and without test parallelization turned on. I should be back with a report later today.

[thecodejunkie]

@blairconrad what was the findings of the experiment? It's no unlikely that StaticConfiguration is the cause of the problem. I've previously added a task to remove the last, remaining parts, of StaticConfiguration in #2659

A while back I also created StaticConfigurationContext to help mitigate race conditions, in tests, that used StaticConfiguration values. It looks like DefaultRouteResolverFixture does not use it.. would be interesting to see if adding it would solve the intermittent test failtures

[blairconrad]

@thecodejunkie, I put my findings in #2703 (I was just coming here to make that note when I saw your comment). Maybe we're better off continuing our conversation there?

[blairconrad]

I see you have!