SingleContextIncludedWithPublish Unit Test passes inconsistently

[joncloud]

Running build.cmd or tests through Visual Studio seems to occasionally fail due to an exception.

Test run on 43e27ee6b42fb6c34ec174df1b46a1b7137a13a8

xUnit.net DNX Runner (32-bit DNX 4.5.1)
  Discovering: Scientist.Test
  Discovered:  Scientist.Test
  Starting:    Scientist.Test
    TheScientistClass+TheScienceMethod.SingleContextIncludedWithPublish [FAIL]
      System.AggregateException : One or more errors occurred.
      ---- System.Exception : Exception of type 'System.Exception' was thrown.
      Stack Trace:
           at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
           at System.Threading.Tasks.Task`1.GetResultCore(Boolean waitCompletionNotification)
           at System.Threading.Tasks.Task`1.get_Result()
        C:\Users\jdber_000\Documents\GitHub\scientist.net\src\Scientist\Scientist.cs(35,0): at GitHub.Scientist.Science[T](String name, Action`1 experiment)
        C:\Users\jdber_000\Documents\GitHub\scientist.net\test\Scientist.Test\ScientistTests.cs(539,0): at TheScientistClass.TheScienceMethod.SingleContextIncludedWithPublish()
        ----- Inner Stack Trace -----
        C:\Users\jdber_000\Documents\GitHub\scientist.net\src\Scientist\Internals\Experiment.cs(13,0): at GitHub.Internals.Experiment`1.<>c.<.cctor>b__35_1(Operation operation, Exception exception)
        C:\Users\jdber_000\Documents\GitHub\scientist.net\src\Scientist\Internals\ExperimentInstance.cs(87,0): at GitHub.Internals.ExperimentInstance`1.<Run>d__12.MoveNext()
  Finished:    Scientist.Test
=== TEST EXECUTION SUMMARY ===
   Scientist.Test  Total: 34, Errors: 0, Failed: 1, Skipped: 0, Time: 0.554s

[haacked]

Is this still ongoing?

[joncloud]

I'll review the code in its current state to see if its applicable, and run some more tests to make sure.

[joncloud]

I ran a couple of dotnet test runs, and got a failure on the second run. It looks like this may be due to xunit running the tests in parallel, and the way we have the tests written we're swapping some of the static context away.

There are a couple of options that I uncovered by adjusting the tests:

• Use CollectionBehavior (assembly level attribute) to disable test parallelization.
• Re-organize the tests so that those that manipulate static context exist within a single collection.
  • Tag test classes so they're in the same Collection (class level attribute)
  • Move test methods so that they're in the same test class (thus same collection)

Quick local test showed that disabling test parallelization increased test time by ~0.1s

[haacked]

@joncloud nice work. What static context are we manipulating?

[joncloud]

Good point - I totally missed that the title of the issue had "Context". Check out the use of the Swap class in the unit test project. We're using it to swap out Scientist.Enabled and Scientist.ResultPublisher.

[haacked]

Oh right. Static state is evil. But in this case, an intentional deal with the devil for simplicity and usability sake.

I like the changes you proposed. Let's do that for now and we can think about whether we want to change anything later.

[joncloud]

I opted for running all tests in series rather than moving all of the tests around, because the of the simplicity. We can definitely expand in the future if we've added enough unit tests that make the run time too slow.

[haacked]

@joncloud that sounds reasonable. These tests are so fast anyways. :smile:

[joncloud]

Fixed on #84.