feat: Generate html report for single scan mode #358

m-rudyk · 2023-12-13T14:49:52Z

Pull Request

Description

added option for signle scan operation to generate html report with conflict files.

Fixes #359

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Code cleanup/refactoring
Documentation update
This change requires a documentation update
CI system update
Test Coverage update

Testing

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.

Test Configuration:

Java: v11
LPVS Release: v1.x.x

Checklist:

My code follows the style guidelines of this project
My code meets the required code coverage for lines (90% and above)
My code meets the required code coverage for branches (80% and above)
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

codecov-commenter · 2023-12-13T14:51:16Z

Codecov Report

Attention: 21 lines in your changes are missing coverage. Please review.

Comparison is base (6298473) 91.34% compared to head (22dce35) 91.01%.

Files	Patch %	Lines
src/main/java/com/lpvs/util/LPVSCommentUtil.java	85.54%	6 Missing and 6 partials ⚠️
.../main/java/com/lpvs/service/LPVSDetectService.java	59.09%	7 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #358      +/-   ##
============================================
- Coverage     91.34%   91.01%   -0.33%     
- Complexity      475      486      +11     
============================================
  Files            48       48              
  Lines          1606     1703      +97     
  Branches        194      206      +12     
============================================
+ Hits           1467     1550      +83     
- Misses           82       89       +7     
- Partials         57       64       +7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Mykola Rudyk <m.rudyk@samsung.com>

m-rudyk · 2023-12-13T15:14:33Z

Example of generated report file:

o-kopysov · 2023-12-13T15:52:49Z

README.md

@@ -271,7 +271,7 @@ Alternatively, you can perform a one-time scan on a specific pull request using
 4.b.2. Execute the single scan with the following command:

   ```bash
-   java -jar -Dgithub.token=<my-token> lpvs-*.jar --github.pull.request=<PR URL>
+   java -jar -Dgithub.token=<my-token> lpvs-*.jar --github.pull.request=<PR URL> --build.html.report=<Path to report folder>


Do we need to specify Path to report folder or Path + Filename?
It's not clear from the description. Pease add more words into Readme.

o-kopysov · 2023-12-13T16:02:06Z

src/main/java/com/lpvs/service/LPVSDetectService.java

-                    log.info(jsonTxt);
-                    log.info("\n\n\n Single scan finished successfully \n\n\n");
+
+                scanossDetectService.runScan(


I can't entirely agree with this line.
The previous logic was to use this.runScan() for the case when several scanners can be used. Current change has hardcoded scanossDetectService.runScan() option which blocks the flexibility of the implementation.

o-kopysov · 2023-12-13T16:27:54Z

src/main/java/com/lpvs/service/LPVSDetectService.java

+                    String report =
+                            LPVSCommentUtil.buildHTMLComment(
+                                    webhookConfig, scanResult, detectedConflicts);
+                    LPVSCommentUtil.saveHTMLToFile(report, buildReport + "/LPVSreport.html");


Maybe it's better to define filename via CLI parameter?

o-kopysov · 2023-12-13T16:28:31Z

src/main/java/com/lpvs/service/LPVSDetectService.java

+                    String report =
+                            LPVSCommentUtil.reportCommentBuilder(
+                                    webhookConfig, scanResult, detectedConflicts);
+                    log.info(report);
                }
            } catch (Exception ex) {
                log.info("\n\n\n Single scan finished with errors \n\n\n");


Maybe log.error is a better choise?

next message is error with reporting. But we may change this as well.

src/main/java/com/lpvs/util/LPVSCommentUtil.java

o-kopysov · 2023-12-13T16:41:27Z

@m-rudyk

Rebase your PR
Check my comments
Improve code coverage:

JaCoCo Test Coverage Summary
Coverage: 89.940%
Branches: 77.586%

o-kopysov

Please check my comments

Signed-off-by: Mykola Rudyk <m.rudyk@samsung.com>

o-kopysov · 2023-12-13T19:06:33Z

@m-rudyk

JaCoCo Test Coverage Summary
Coverage: 91.371%
Branches: 78.007%

Signed-off-by: Mykola Rudyk <m.rudyk@samsung.com>

tiokim

PTAL

tiokim · 2023-12-13T23:53:52Z

src/main/java/com/lpvs/service/LPVSDetectService.java

-                    // ToDo: form html report and console output
-                    log.info(jsonTxt);
-                    log.info("\n\n\n Single scan finished successfully \n\n\n");
+                List<LPVSFile> scanResult = scanossDetectService.checkLicenses(webhookConfig);


I think the code can be revised like below.

List<LPVSFile> scanResult = this.runScan(webhookConfig, LPVSDetectService.getPathByPullRequest(webhookConfig));

tiokim · 2023-12-13T23:55:48Z

src/main/java/com/lpvs/service/LPVSDetectService.java

    @Autowired private ApplicationEventPublisher eventPublisher;

    @Value("${github.pull.request:}")
    private String trigger;

+    @Value("${build.html.report:}")
+    private String buildReport;


What about changing the name to htmlReport?

tiokim · 2023-12-14T00:09:57Z

src/main/java/com/lpvs/service/LPVSDetectService.java

+                List<LPVSLicenseService.Conflict<String, String>> detectedConflicts =
+                        licenseService.findConflicts(webhookConfig, scanResult);
+
+                if (buildReport != null && !HtmlUtils.htmlEscape(buildReport).equals("")) {


Why do we need to run HtmlUtils.htmlEscape(buildReport)?

o-kopysov · 2023-12-14T06:24:24Z

src/main/java/com/lpvs/util/LPVSCommentUtil.java

@@ -66,4 +74,122 @@ public static String getMatchedLinesAsLink(
        log.debug("MatchedLines: " + matchedLines);
        return matchedLines;
    }
+
+    public static String reportCommentBuilder(


Please add Javadoc comments for all new functions.

Signed-off-by: Mykola Rudyk <m.rudyk@samsung.com>

o-kopysov

LGTM

o-kopysov · 2023-12-15T09:01:31Z

@m-rudyk Please resolve merge conflicts.

Signed-off-by: m-rudyk <121865672+m-rudyk@users.noreply.github.com>

m-rudyk force-pushed the m.rudyk/generate_html_report branch from cd6698e to 2103c2a Compare December 13, 2023 15:07

m-rudyk added 3 commits December 13, 2023 17:08

doc: add command description to readme

7240ea0

Signed-off-by: Mykola Rudyk <m.rudyk@samsung.com>

feat: add html reporting option

1f11305

Signed-off-by: Mykola Rudyk <m.rudyk@samsung.com>

fix: fix style errors

9014e0d

Signed-off-by: Mykola Rudyk <m.rudyk@samsung.com>

m-rudyk force-pushed the m.rudyk/generate_html_report branch from 2103c2a to 9014e0d Compare December 13, 2023 15:08

m-rudyk marked this pull request as ready for review December 13, 2023 15:14

m-rudyk requested review from tiokim, MoonkiHong, o-kopysov and tdrozdovsky as code owners December 13, 2023 15:14

m-rudyk requested review from o-konoval and v-kerimov December 13, 2023 15:18

o-kopysov changed the title ~~M.rudyk/generate html report~~ feat: Generate html report for single scan mode Dec 13, 2023

o-kopysov reviewed Dec 13, 2023

View reviewed changes

src/main/java/com/lpvs/util/LPVSCommentUtil.java Show resolved Hide resolved

o-kopysov self-requested a review December 13, 2023 16:41

o-kopysov requested changes Dec 13, 2023

View reviewed changes

o-kopysov assigned m-rudyk Dec 13, 2023

o-kopysov added the new feature label Dec 13, 2023

o-kopysov added this to the v1.3.0 milestone Dec 13, 2023

m-rudyk force-pushed the m.rudyk/generate_html_report branch from 4bf5647 to 217f3ef Compare December 13, 2023 17:20

fix: apply changes after review comment

12a8f1b

Signed-off-by: Mykola Rudyk <m.rudyk@samsung.com>

m-rudyk force-pushed the m.rudyk/generate_html_report branch 2 times, most recently from 45c36f7 to 478a7d8 Compare December 13, 2023 18:51

m-rudyk force-pushed the m.rudyk/generate_html_report branch 4 times, most recently from 8bc02c0 to a889502 Compare December 13, 2023 22:38

m-rudyk requested a review from o-kopysov December 13, 2023 22:39

test: udpate unit tests

7965aca

Signed-off-by: Mykola Rudyk <m.rudyk@samsung.com>

m-rudyk force-pushed the m.rudyk/generate_html_report branch from 449bf24 to 7965aca Compare December 13, 2023 22:40

tiokim reviewed Dec 14, 2023

View reviewed changes

o-kopysov reviewed Dec 14, 2023

View reviewed changes

m-rudyk and others added 2 commits December 14, 2023 09:00

Merge branch 'Samsung:main' into m.rudyk/generate_html_report

af0d2ce

doc: add javadoc description

18f0fd9

Signed-off-by: Mykola Rudyk <m.rudyk@samsung.com>

m-rudyk force-pushed the m.rudyk/generate_html_report branch from aee75c7 to 18f0fd9 Compare December 14, 2023 07:29

m-rudyk requested review from tiokim and o-kopysov December 14, 2023 07:33

o-kopysov approved these changes Dec 15, 2023

View reviewed changes

m-rudyk force-pushed the m.rudyk/generate_html_report branch 2 times, most recently from d1279ea to 6b261c7 Compare December 18, 2023 11:25

Merge branch 'main' into m.rudyk/generate_html_report

22dce35

Signed-off-by: m-rudyk <121865672+m-rudyk@users.noreply.github.com>

m-rudyk force-pushed the m.rudyk/generate_html_report branch from 6b261c7 to 22dce35 Compare December 18, 2023 11:28

v-kerimov approved these changes Dec 18, 2023

View reviewed changes

m-rudyk merged commit 2cfc37e into Samsung:main Dec 18, 2023
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Generate html report for single scan mode #358

feat: Generate html report for single scan mode #358

m-rudyk commented Dec 13, 2023 •

edited by o-kopysov

codecov-commenter commented Dec 13, 2023 •

edited

m-rudyk commented Dec 13, 2023

o-kopysov Dec 13, 2023

o-kopysov Dec 13, 2023

m-rudyk Dec 13, 2023

o-kopysov Dec 13, 2023

o-kopysov Dec 13, 2023

m-rudyk Dec 13, 2023

o-kopysov commented Dec 13, 2023

o-kopysov left a comment

o-kopysov commented Dec 13, 2023

tiokim left a comment

tiokim Dec 13, 2023

tiokim Dec 13, 2023

tiokim Dec 14, 2023

o-kopysov Dec 14, 2023

o-kopysov left a comment

o-kopysov commented Dec 15, 2023

feat: Generate html report for single scan mode #358

feat: Generate html report for single scan mode #358

Conversation

m-rudyk commented Dec 13, 2023 • edited by o-kopysov

Pull Request

Description

Type of change

Testing

Checklist:

codecov-commenter commented Dec 13, 2023 • edited

Codecov Report

m-rudyk commented Dec 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

o-kopysov commented Dec 13, 2023

o-kopysov left a comment

Choose a reason for hiding this comment

o-kopysov commented Dec 13, 2023

tiokim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

o-kopysov left a comment

Choose a reason for hiding this comment

o-kopysov commented Dec 15, 2023

m-rudyk commented Dec 13, 2023 •

edited by o-kopysov

codecov-commenter commented Dec 13, 2023 •

edited