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

Easy report generation #247

Merged
merged 1 commit into from
May 25, 2019
Merged

Conversation

karanjitsingh
Copy link
Contributor

@karanjitsingh karanjitsingh commented May 24, 2019

  • Expose ReportConfigurationBuilder
    • Refactor code to be able to create ReportConfiguration from a dictionary
  • Null check for RiskHotspotAnalysisResult in ReportBuilderBase
  • Expose ReportContext ReportGenerator CachingFileReader IFileReader

Creating reports using the SDK becomes as simple as:

using ...

namespace htmlreport
{
    class Program
    {
        static void Main(string[] args)
        {
            var result = ParseCoverageFiles(new List<string>() { @"D:\CodeCoverageReport_8440364\coverage.xml" });
            CreateHTMLReport(result, DateTime.Now);
        }

        private static ParserResult ParseCoverageFiles(List<string> coverageFiles)
        {
            CoverageReportParser parser = new CoverageReportParser(
                1,
                new string[] { },
                new DefaultFilter(new string[] { }),
                new DefaultFilter(new string[] { }),
                new DefaultFilter(new string[] { }));

            ReadOnlyCollection<string> collection = new ReadOnlyCollection<string>(coverageFiles);
            return parser.ParseFiles(collection);
        }

        private static void CreateHTMLReport(ParserResult parserResult, DateTime executionTime)
        {
            var summaryResult = new SummaryResult(parserResult);
            var builder = new HtmlInlineAzurePipelinesReportBuilder();

            var folder = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
            Directory.CreateDirectory(folder);

            var config = new ReportConfigurationBuilder().Create(new Dictionary<string, string>() {
                { "targetdir", folder },
                { "reporttypes", "HtmlInline_AzurePipelines" }
            });

            var reportContext = new ReportContext(config, new Settings());

            builder.ReportContext = reportContext;

            new ReportGenerator(
                new CachingFileReader(reportContext.Settings.CachingDuringOfRemoteFilesInMinutes),
                parserResult,
                new List<IReportBuilder>() { builder } )
                    .CreateReport(false, null, executionTime, reportContext.ReportConfiguration.Tag);
        }
    }
}

@@ -237,32 +237,35 @@ public virtual void CreateSummaryReport(IReportRenderer reportRenderer, SummaryR
reportRenderer.MetricsTable(new[] { methodMetric });
}

if (reportRenderer.SupportsRiskHotsSpots
&& this.ReportContext.RiskHotspotAnalysisResult.CodeCodeQualityMetricsAvailable)
if (this.ReportContext.RiskHotspotAnalysisResult != null)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gtihub diff seems weird

image

@danielpalme
Copy link
Owner

I think you can achieve the same result, with much less code and fewer changes in ReportGenerator.
Only the changes to ReportConfigurationBuilder are required:

The following sample demonstrates how to use the higher level APIs:

using ...

namespace htmlreport
{
    class Program
    {
        static void Main(string[] args)
        {
            CreateHTMLReport(new List<string>() { @"D:\CodeCoverageReport_8440364\coverage.xml" });
        }

        private static void CreateHTMLReport(IEnumerable<string> coverageFiles)
        {
            var folder = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
            Directory.CreateDirectory(folder);

            var reportConfigurationBuilder = new ReportConfigurationBuilder();
            ReportConfiguration configuration = reportConfigurationBuilder.Create(new Dictionary<string, string>() {
                { "reports", string.Join(";", coverageFiles) }
                { "targetdir", folder },
                { "reporttypes", "HtmlInline_AzurePipelines" }
            });

            new Generator().GenerateReport(configuration);
        }
    }
}

What do you think?

@karanjitsingh
Copy link
Contributor Author

My original idea was to modify Generator.cs itself. I wanted to separate parsing and report creation for scenarios where only parsing is required.

@danielpalme danielpalme merged commit 50e029c into danielpalme:master May 25, 2019
@danielpalme
Copy link
Owner

My recommendation is to:

  • Use CoverageReportParser to parse coverage files and then do you own processing based on the ParserResult.
  • Use the approach from above to generate HTML reports.

Don't use the (currently internal classes). If they become public and you start using them, it would become very hard for me to change the internal behavior without breaking your code.

@karanjitsingh
Copy link
Contributor Author

karanjitsingh commented May 26, 2019

@danielpalme, is there a way to generate HTML reports based on the ParserResult that I have? For example if I simply go through Generator it will parse the files for me instead of using my ParserResult instance.

My ask here is to decouple these two operations.

I can think of two ways of doing this without exposing internal classes,

  • Separate Parsing and report generation in Generator itself, and GenerateReport calls into both of these to continue working the same way. This way Generator can expose methods for decoupled parsing and report generation as well.
  • Another way of doing this is by having an overloaded implementation of GenerateReport to accept an instance of ParserResult and use that instead of parsing on its own. (I personally prefer this one)

It's possible in my scenario that first we parse the files and decide later on in time whether to create HTML reports or not, hence decoupling the two operations would make doing that simpler.

Let me know if this makes sense and I can raise a PR.

@danielpalme
Copy link
Owner

danielpalme commented May 26, 2019

I will have a look at this tomorrow.

@danielpalme
Copy link
Owner

danielpalme commented May 27, 2019

I added a two new overloads of GenerateReport:

public void GenerateReport(

public void GenerateReport(

This (pre-)release contains the new methods:
https://www.nuget.org/packages/ReportGenerator.Core/4.1.9-rc2

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