Improve instrumentation tests #420

MarcoRossignoli · 2019-05-23T11:43:57Z

contributes to #387

@tonerdo @petli with this PR we've a way to test the "reality" of instrumentation and coverage calculation.

What we have:

a new helper class RemoteExecutor.Invoke that allow us to run a "lambda" in remote process to avoid issue with statics and file locks 🚀 🚀 🚀
new possible assertions on line/branches coverage
we can "debug" all process, no more repro local build and reference+Debug.Launch...we can add "case" on test code and reproduce, fix and keep and run on every CI, I think this will improve the quality and keep control on new "compiler versions" crazyness autogencode.
to allow this I added the infrastructure for a DI pattern, I needed only RetryHelper and IProcessExitHandler injection for now...but with this in place we can improve testability of all components that will become injectables ❤️ ❤️ ❤️ !

MarcoRossignoli · 2019-05-23T11:46:53Z

src/coverlet.core/DependencyInjection.cs

@@ -0,0 +1,34 @@
+using System;


@tonerdo this is a key class, setup a "container" with all injectable services(for now only retry helper).
We have some "static" code(methods) so we need a domain wide model DependencyInjection.Current provide access to all services.
I used DI framework used by ASP.NET Code inside Microsoft.Extensions.DependencyInjection https://github.com/aspnet/Extensions

MarcoRossignoli · 2019-05-23T11:48:00Z

src/coverlet.core/DependencyInjection.cs

+            }
+        }
+
+        public static void Set(IServiceProvider serviceProvider)


this method needed to "plug new services collection" used by testing to pass mocked IRetryHelper aware of process exit and file locking.

MarcoRossignoli · 2019-05-23T11:48:45Z

src/coverlet.core/Helpers/InstrumentationHelper.cs

@@ -7,8 +7,9 @@
 using System.Reflection;


this file is the only file changed to use "container" low cost change.

MarcoRossignoli · 2019-05-23T11:49:40Z

src/coverlet.core/Helpers/RetryHelper.cs

@@ -1,3 +1,4 @@
+using Coverlet.Core.Abstracts;


Here we simply make RetryHelper an injectable "instance" type that implements IRetryHelper.

MarcoRossignoli · 2019-05-23T11:51:47Z

src/coverlet.core/coverlet.core.csproj

    <PackageReference Include="Mono.Cecil" Version="0.10.1" />
-    <PackageReference Include="Newtonsoft.Json" Version="9.0.1" />
+    <PackageReference Include="Newtonsoft.Json" Version="10.0.1" />


I change this to 10 because Newtosolft has got a bug on 9.0.1 and cannot use TypeConverter in a correct way, this lead to exception during serialization of CoveragePrepareResult in testing.
MSbuild/VSTest collectors does not load this version but version 10.0.1 this is the reason why it works today(unexpected 😄).
Maybe @vagisha-nidhi has got some details on this.

MarcoRossignoli · 2019-05-23T11:53:37Z

test/coverlet.core.tests/CoverageTests.cs

@@ -54,5 +60,48 @@ public void TestCoverageWithTestAssembly()

            directory.Delete(true);
        }
+
+        [Fact]
+        public void Condition_If()


@tonerdo @petli start from here!
This is the code we need to write to test!

MarcoRossignoli · 2019-05-23T11:54:31Z

test/coverlet.core.tests/Samples/Instrumentation.cs

+
+namespace Coverlet.Core.Samples.Tests
+{
+    public class Conditions


Added new file for new "class" of tests.
Wrote a "pilot" test.

MarcoRossignoli · 2019-05-23T12:06:56Z

I need to fix post build copy on linux, let me fix

MarcoRossignoli · 2019-05-23T13:51:04Z

Fail on unix...I'm investigating.

MarcoRossignoli · 2019-05-23T15:23:33Z

Found the issue, race between RestoreOriginalModules and RestoreOriginalModule on msbuild/console/collector are in same AppDomain on test nope so they race on Copy/Delete on locked files.
I'll fix it.

MarcoRossignoli · 2019-05-23T15:53:46Z

src/coverlet.core/Abstracts/IProcessExitHandler.cs

+
+namespace Coverlet.Core.Abstracts
+{
+    public interface IProcessExitHandler


We need this service to plug a fake in child process to avoid race condition on restore module.
In normal condition(no exception) we restore modules during coverage calculation, after @ViktorHofer update we restore also other instrumented module on exit, on msbuild/console/collector we use a shared dictionary but on test we instrument and run on child process so parent/child race on restore/delete files.
On child test we'll do nop.

MarcoRossignoli · 2019-05-23T16:00:55Z

Bonus, improved coverage
Pre

 +---------------+--------+--------+--------+
  | Module        | Line   | Branch | Method |
  +---------------+--------+--------+--------+
  | coverlet.core | 76,03% | 67,42% | 83,03% |
  +---------------+--------+--------+--------+

  +---------+--------+--------+--------+
  |         | Line   | Branch | Method |
  +---------+--------+--------+--------+
  | Total   | 76,03% | 67,42% | 83,03% |
  +---------+--------+--------+--------+
  | Average | 76,03% | 67,42% | 83,03% |
  +---------+--------+--------+--------+

Post

  +---------------+--------+--------+--------+
  | Module        | Line   | Branch | Method |
  +---------------+--------+--------+--------+
  | coverlet.core | 80,98% | 71,1%  | 88,82% |
  +---------------+--------+--------+--------+

  +---------+--------+--------+--------+
  |         | Line   | Branch | Method |
  +---------+--------+--------+--------+
  | Total   | 80,98% | 71,1%  | 88,82% |
  +---------+--------+--------+--------+
  | Average | 80,98% | 71,1%  | 88,82% |
  +---------+--------+--------+--------+

@tonerdo we lost codecov report.

MarcoRossignoli · 2019-05-25T07:19:10Z

@tonerdo great thank's for WIP!
Ready to review!

MarcoRossignoli · 2019-08-06T12:24:07Z

@tonerdo can you review this?So we can go on with "core instrumentation" fix and improve test coverage .

src/coverlet.core/Abstracts/IRetryHelper.cs

tonerdo

LGTM!

MarcoRossignoli requested review from tonerdo and petli May 23, 2019 11:44

MarcoRossignoli commented May 23, 2019

View reviewed changes

MarcoRossignoli changed the title ~~Improve instrumentation tests~~ [WIP]Improve instrumentation tests May 23, 2019

MarcoRossignoli changed the title ~~[WIP]Improve instrumentation tests~~ Improve instrumentation tests May 23, 2019

improve instrumentation tests

40450fc

MarcoRossignoli force-pushed the remoteinvoker branch from 22d4c20 to 40450fc Compare May 23, 2019 15:48

MarcoRossignoli commented May 23, 2019

View reviewed changes

tonerdo changed the title ~~Improve instrumentation tests~~ [WIP] Improve instrumentation tests May 24, 2019

MarcoRossignoli changed the title ~~[WIP] Improve instrumentation tests~~ Improve instrumentation tests May 25, 2019

MarcoRossignoli mentioned this pull request May 25, 2019

Make build more coherent with same "dotnet sdk" on all CI machine. #396

Closed

This was referenced Jun 6, 2019

Update newtosoft reference #453

Merged

Improve instrumentation testability #387

Closed

MarcoRossignoli added the enhancement General enhancement request label Jun 11, 2019

MarcoRossignoli mentioned this pull request Aug 1, 2019

Coverage does not distinguish between property getters and setters #507

Closed

MarcoRossignoli added 3 commits August 6, 2019 13:48

merge

df8a2fe

fix build

b05e6ba

fix using

f1c6516

ViktorHofer reviewed Aug 7, 2019

View reviewed changes

src/coverlet.core/Abstracts/IRetryHelper.cs Outdated Show resolved Hide resolved

address PR feedback

f280bf7

petli approved these changes Aug 8, 2019

View reviewed changes

MarcoRossignoli added 2 commits August 8, 2019 16:15

merge master

6a0fa55

fix after merge

6541da5

petli approved these changes Aug 8, 2019

View reviewed changes

tonerdo approved these changes Aug 9, 2019

View reviewed changes

MarcoRossignoli merged commit cca1668 into coverlet-coverage:master Aug 9, 2019

MarcoRossignoli deleted the remoteinvoker branch August 9, 2019 18:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve instrumentation tests #420

Improve instrumentation tests #420

MarcoRossignoli commented May 23, 2019 •

edited

Loading

MarcoRossignoli May 23, 2019

MarcoRossignoli May 23, 2019

MarcoRossignoli May 23, 2019

MarcoRossignoli May 23, 2019 •

edited

Loading

MarcoRossignoli May 23, 2019 •

edited

Loading

MarcoRossignoli May 23, 2019

MarcoRossignoli May 23, 2019 •

edited

Loading

MarcoRossignoli commented May 23, 2019

MarcoRossignoli commented May 23, 2019

MarcoRossignoli commented May 23, 2019

MarcoRossignoli May 23, 2019

MarcoRossignoli commented May 23, 2019 •

edited

Loading

MarcoRossignoli commented May 25, 2019

MarcoRossignoli commented Aug 6, 2019

tonerdo left a comment

Improve instrumentation tests #420

Improve instrumentation tests #420

Conversation

MarcoRossignoli commented May 23, 2019 • edited Loading

MarcoRossignoli May 23, 2019

Choose a reason for hiding this comment

MarcoRossignoli May 23, 2019

Choose a reason for hiding this comment

MarcoRossignoli May 23, 2019

Choose a reason for hiding this comment

MarcoRossignoli May 23, 2019 • edited Loading

Choose a reason for hiding this comment

MarcoRossignoli May 23, 2019 • edited Loading

Choose a reason for hiding this comment

MarcoRossignoli May 23, 2019

Choose a reason for hiding this comment

MarcoRossignoli May 23, 2019 • edited Loading

Choose a reason for hiding this comment

MarcoRossignoli commented May 23, 2019

MarcoRossignoli commented May 23, 2019

MarcoRossignoli commented May 23, 2019

MarcoRossignoli May 23, 2019

Choose a reason for hiding this comment

MarcoRossignoli commented May 23, 2019 • edited Loading

MarcoRossignoli commented May 25, 2019

MarcoRossignoli commented Aug 6, 2019

tonerdo left a comment

Choose a reason for hiding this comment

MarcoRossignoli commented May 23, 2019 •

edited

Loading

MarcoRossignoli May 23, 2019 •

edited

Loading

MarcoRossignoli May 23, 2019 •

edited

Loading

MarcoRossignoli May 23, 2019 •

edited

Loading

MarcoRossignoli commented May 23, 2019 •

edited

Loading