Fix ObservableTests

[sharwell]

Fixes #249

The root cause of the test failures is the following:

1. This sanity check failed with an exception here, seemingly due to an addDomain field which is possible related to this TODO (e.g. if the appDomain field doesn't appear in the baseline files due to some omission in the TPL parser; the details here are totally outside my current understanding)
2. This exception handler resulted in a string being assigned to a field
3. The string value failed to convert to a long
4. The exception was caught in Dispatch, bypassing the code which assigns eventRecord back to null
5. When the event was eventually unsubscribed, this assertion failed causing the test to hang

This pull request modifies the event dispatch mechanism to ensure eventRecord is set back to null after dispatch in all cases.

[codecov-io]

Codecov Report

Merging #257 into master will increase coverage by 0.29%. The diff coverage is 65.83%.

@@            Coverage Diff             @@
##           master     #257      +/-   ##
==========================================
+ Coverage   17.44%   17.73%   +0.29%     
==========================================
  Files         213      213              
  Lines      123790   123869      +79     
  Branches    11971    11977       +6     
==========================================
+ Hits        21590    21964     +374     
+ Misses     101189   101042     -147     
+ Partials     1011      863     -148

Flag	Coverage Δ
#2017	17.73% <65.83%> (+0.29%)	:arrow_up:
#Debug	17.73% <65.83%> (+0.29%)	:arrow_up:
#Release	?

Impacted Files	Coverage Δ
src/TraceEvent/Ctf/CtfTraceEventSource.cs	0% <0%> (ø)	:arrow_up:
src/TraceEvent/ETWReloggerTraceEventSource.cs	0% <0%> (ø)	:arrow_up:
src/TraceEvent/TraceEvent.Tests/DispatchTests.cs	40.83% <100%> (+0.66%)	:arrow_up:
src/TraceEvent/TraceEvent.cs	66.39% <100%> (+3.16%)	:arrow_up:
src/TraceEvent/TraceEvent.Tests/ObservableTests.cs	96.62% <100%> (+96.62%)	:arrow_up:
src/TraceEvent/TraceLog.cs	61.24% <41.37%> (+0.2%)	:arrow_up:
src/TraceEvent/ETWTraceEventSource.cs	49.2% <81.81%> (+2.09%)	:arrow_up:
src/PerfView/StackViewer/PerfDataGrid.xaml.cs	34.02% <0%> (ø)	:arrow_up:
...c/TraceEvent/Parsers/ClrPrivateTraceEventParser.cs	19.58% <0%> (+0.03%)	:arrow_up:
... and 12 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ed9cc8d...a43cce3. Read the comment docs.

[vancem]

I need to think about this more.

I really want the dispatch code to be as lean (and simple) as possible. Thus I really don't want us establishing an invariant that anEvent.eventRecord should be null on return from processing Indeed in dispatch we say

    anEvent.eventRecord = null;      // Technically not needed but detects user errors sooner. 

because it is a nice to have (you get AVs if it is not set) but it is not essential to the code. My expectation is that eventRecord should always be set in dispatch and while it may have a stale value after processing, that should not matter.

[sharwell]

Updated after repository rewrite