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 an NCover-compatible XML report output #7

Merged
merged 2 commits into from
Jan 18, 2018

Conversation

jonjomckay
Copy link
Contributor

I had to change the structure of the output coverage JSON to make this fit (so it's not compatible with previously-output coverage), but now people can use something like ReportGenerator to generate HTML reports :)

@lucaslorentz
Copy link
Owner

That's awesome! I will review it later. Thanks for contributing!

Copy link
Owner

@lucaslorentz lucaslorentz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some tests and it is completely functional and reports generated with ReportGenerator looks good.

No blocker in the comments.

@@ -9,16 +9,16 @@ public static void Execute(InstrumentationResult result)
{
foreach (var assembly in result.Assemblies)
{
if (File.Exists(assembly.BackupFile))
if (File.Exists(assembly.Value.BackupFile))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can avoid .Value by changing the foreach to foreach (var assembly in result.Assemblies.Values)

@@ -154,13 +157,17 @@ private void InstrumentAssembly(string assemblyFile)

var instructionId = ++id;

result.AddInstruction(sourceRelativePath, new InstrumentedInstruction
result.Assemblies[assemblyDefinition.Name.Name].AddInstruction(sourceRelativePath, new InstrumentedInstruction
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer that the method AddInstrumentedAssembly returns the InstrumentedAssembly object.

Then we don't need to access the assembly by dictionary key here.

@jonjomckay
Copy link
Contributor Author

Thanks for the feedback @lucaslorentz - I made the changes!

@lucaslorentz lucaslorentz merged commit babaaf7 into lucaslorentz:master Jan 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants