Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a method to unload instrumentation of files without generating the final report #1694

Open
JosephSun2003 opened this issue Aug 26, 2024 · 3 comments
Labels
feature-request New feature request untriaged To be investigated

Comments

@JosephSun2003
Copy link
Contributor

Hello!

My name is Joseph Sun and I’m an intern at Geotab tasked with generalising my supervisor’s hack of Coverlet. We believe the changes we're suggesting will help with use cases of Coverlet that spread testing execution over mutliple parallel jobs.

To briefly explain, Geotab is a telematics company focused on providing fleet management solutions for both private and public sector organizations. The company utilizes Coverlet to assist with its continuous integration and development, primarily calculating and tracking unit test coverage.

The Problem

The problem encountered with the incorporation of Coverlet is the needlessly redundant processing when jobs are parallelized. Specifically, the execution of the unloading of instrumentation and aggregation of Coverlet’s test coverage result.

Since instrumentation unloading and Coverlet’s result aggregation are set to always occur one after the other, parallelized jobs will run multiple instances of report generation simultaneously, these results would then be used to further report generation to aggregate the already aggregated results.

As such, the simultaneous report generation is an unnecessary process as one instance of report generation can be run instead, rather than the multiple in parallel that require the further aggregation anyways.

GeoTab’s Hack

The hack implemented by my supervisor, then, is to invoke the unloading of instrumentation using the .Net Reflection library.

Due to how the Reflection library scans assemblies and source files directly, the Reflection library is able to invoke the “UnloadModule” method found in “ModuleTrackerTemplate.” The Reflection library is able to bypass the injected nature of the “ModuleTrackerTemplate,” which would make invoking “UnloadModule” normally extremely difficult.

Problems with the Hack

The main problem with this hack is the long term maintenance of Geotab’s Coverlet integration. Due to how Reflection inherently works, any changes to Coverlet involving the unloading of instrumentation may result in numerous issues for Geotab.

As such, Geotab, and many users who utilize parallelization, would benefit from Coverlet being able to work more seamlessly with systems, projects and others using parrellism in code testing.

Planned changes

As noted earlier, Coverlet’s current implementation makes integration with testing utilizing parallelization unnecessarily resource intensive. The hack used in Geotab being an example of an ad hoc solution that has questionable long term sustainability.

Desired Change Results

The goal of these changes are as follows:

  • Enable developers to invoke a method to unload instrumentation of assemblies
  • Maintain existing execution operations for testing in projects that are still executed serially
  • Add required unit tests to ensure functionality

The goal of the changes would create the following logic flow:

Change Details

The following will note down on where major changes will occur:

Coverage.cs

The primary change in the Coverage.cs file will be the extraction of the unloading of instrumented files into its own method.

Additionally, for methods such as GetCoverageResult(), some modifications will be made to check whether or not specified modules have already been unloaded due to an invocation of the new method.

This will preserve the existing logic flow while enabling Coverlet to be more compatible with parallelized testing.

Unit Tests

The test files will see many changes, primarily additions to already existing test files.

These additional tests will be focused on verifying the functionality of separately unloading modules and the additional logic branch of modules being unloaded for the GetCoverageResult() method. That said, most of these changes will occur within CoverageTests.cs.

Existing tests of GetCoverageResult() will need little to no modification since the changes are intended to expand on the existing logic flow of Coverlet’s result generation.

Coverlet.Console

The files under coverlet.console will see minor changes to work with the new added method. Specifically, coverlet.console will see changes that will allow developers to manually invoke the unloading of module instrumentation.

Benefits

The benefits of these changes will be twofold:

  1. Existing testing that currently integrate coverlet should not be affected by these changes
  2. Coverlet will have better compatibility with testing utilizing parallelization
    • This benefit is highlighted in Geotab’s use case as it avoid needing an ad hoc solution

Let me know if there’s anything unclear, I’ll be happy to further discuss these changes.

@github-actions github-actions bot added the untriaged To be investigated label Aug 26, 2024
@ghost
Copy link

ghost commented Aug 26, 2024

you're right, idk why it was sent

@ghost
Copy link

ghost commented Aug 26, 2024

i just saw my account keep sending this comment to tens of issues and discussions like canonballs

@JosephSun2003
Copy link
Contributor Author

Hello, I've noticed recently the tag "Needs more info" was added. What information is exactly missing or needed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature request untriaged To be investigated
Projects
None yet
Development

No branches or pull requests

4 participants
@Bertk @JosephSun2003 and others