-
Notifications
You must be signed in to change notification settings - Fork 39
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
Extract common Library to enable consumption from other tools #31
Comments
That sounds great. Splitting assembly is a very good move for Minicover. One simple approach would be:
A more complete approach would also split reports into:
Then some reports can be managed on separate repositories by different maintainers. What do you think? |
I'm looking forward to see a Cake integration after splitting assemblies. |
Thanks! In regards to your comments:
I need to familiarize myself with the codebase more but that sounds like a sane approach to me.
I'm not sure what your plans are report-wise bit I would be concerned about assembly sprawl if you go this route. Sure, most end users will only be producing one or two types of reports and thus need to add one or two additional references, but it adds a lot of complexity to maintain so many assemblies and nuget packages. The other thing to consider is I'm not entirely familiar with how the DotNetCliTool references work. Can you have optional dependencies for the tool? Do you have to do discovery manually? Or would the solution be "If you want third-party reports, use the tool manually or via the cake addin"? I know on the cake side of things they have
This provides a route to eventually include popular report types while still freeing developers to develop their own report types. In my specific use case, I would reference |
About reports. I think the simplest solution would be each report be a separate tool that reads Minicover output files and generates the report. We could provide a library to support those separate tools. It is indeed something like "If you want third-party reports, use the tool manually or via the cake addin". I liked your suggestions and comparison to Cake. I will keep reports together for now. I think I will try to create a PR with those changes. Then we can discuss further on the PR, I will add you as reviewer. |
I see, that makes sense to me. Thanks for the speedy replies! |
I was able to get it working using the const string SOLUTION = "./Harbormaster.sln";
const string MINICOVER = "./minicover/minicover.csproj";
// ...
Task("Restore")
.Does(() =>
{
DotNetCoreRestore(SOLUTION);
DotNetCoreRestore(MINICOVER);
});
// ...
Task("Test")
.IsDependentOn("Clean::Test")
.IsDependentOn("Build")
.Does(() =>
{
DotNetCoreTool(MINICOVER, "minicover", "instrument --workdir ../ --assemblies test/**/bin/**/*.dll --exclude-assemblies test/**/bin/**/*Test*.dll --sources src/**/*.cs");
DotNetCoreTool(MINICOVER, "minicover", "reset");
foreach(var project in GetFiles("./test/**/*.csproj"))
{
DotNetCoreTest(project.FullPath, new DotNetCoreTestSettings
{
Configuration = configuration,
NoRestore = true,
NoBuild = true
});
}
DotNetCoreTool(MINICOVER, "minicover", "uninstrument --workdir ../");
DotNetCoreTool(MINICOVER, "minicover", "report --workdir ../ --threshold 90");
}); Prints some nice basic results:
|
While I agree the libs should be splitted, you know you can ref the nuget as a packagereference and then use the code directly. |
That's great! Build script looks very clean! I'm working on the assembly split. Then I will work on having release versions. |
@bhugot Good point, I guess I got caught up trying to do it the way a lot of other cake addins are written in that they call out to the tool (which isn't restorable by cake). I'll have to look into that. |
@bhugot Actually, now that I think about it, that won't work (at least not as a standard cake addin). Cake addins target
|
@nlowe oh yeah you right didn't thought about that |
@lucaslorentz @nlowe |
@day01 That's definitely something to consider, but I'm not aware of any other cake addin that works like that, and I would hate to have people who want to use MiniCover but not cake to pull in a transient reference to |
@nlowe, At the early stage of Cake nuget had process wrapper. |
So I got around to hacking together Cake.MiniCover which wraps the #addin "Cake.MiniCover"
SetMiniCoverToolsProject("./minicover/minicover.csproj");
// ...
Task("Test")
.Does(() =>
{
MiniCover(tool =>
tool.DotNetCoreTest("./test/Sample.MyLib.Tests/Sample.MyLib.Tests.csproj", new DotNetCoreTestSettings
{
Configuration = configuration,
NoRestore = true,
NoBuild = true
}),
new MiniCoverSettings()
.WithAssembliesMatching("./test/**/*.dll")
.WithSourcesMatching("./src/**/*.cs")
.GenerateReport(ReportType.CONSOLE | ReportType.XML)
.WithNonFatalThreshold()
);
});
// ... This will print coverage with the default threshold of 90%, but will not fail the build. This is useful if you have a different tool parsing the coverage and blocking PR's. The default behavior will still fail the build, or you can be explicit and call |
@nlowe Looks really good. |
@lucaslorentz I think a github project would be great to plan changes |
What is the status on this? As soon as this is done, I can start using (and contributing to) Minicover! |
@ffMathy Can you please take a look at the readme? Let me know if it's what you expect: |
I love it! 😍 |
I want to write a Cake addin for MiniCover like the one that exists for OpenCover. I cannot easily instruct users to add MiniCover as a tool as the way cake downloads tools from Nuget is not compatible with the
DotnetCliTool
package type. I could tell users to create a tools project as instructed in the README and pass that to my addin, but I think an easier solution is if we separate the actual instrumentation and reporting logic from the dotnet cli tool frontend. This would allow me to reference it as a dependency in my package and enable users to get simple coverage with something like:Then, generating coverage becomes as simple as
./build.sh -t Coverage # Or on Windows: ./build.ps1 -t Coverage
The text was updated successfully, but these errors were encountered: