Add cli option to group reporter output. Closes #38 #92

jcsmurph · 2023-11-20T17:13:30Z

Added the option to group the report output by directory, filetype, or pass/fail using -groupby. The option can use one or more commands to organize the report output.

Add -groupby command to group output by file type, pass/fail, and directory

Add: Validation for the groupby command and return error if value passed is not a valid command Add: Allow for multiple values for the groupby command

Add: Added tests for cli_test.go and validator_test.go

Reformatted the groupBy functions and groupBy validation

Added more test files for testing purposes

Updated the groupings to be done in reverse order. Updated the readme with information on the groupby command

Fixed the groupby directory function to group by the last directory in the path

Updated CLI tests

Fixed the cleanString funciton comment

jcsmurph · 2023-11-20T17:18:38Z

closes #38

kehoecj

@jcsmurph Thank you for the PR! I ran the Github action and it looks like there are a couple go fmt issues

README.md

pkg/cli/cli_test.go

pkg/cli/group_output.go

jcsmurph · 2023-11-20T18:39:12Z

@kehoecj Seems my indentation is off in a few places. I will fix those along with @jd4235 suggestions once the review is completed.

jd4235 · 2023-11-20T18:44:55Z

I'll re-review once comments are incorporated

Co-authored-by: Jamie Davidson <51518462+jd4235@users.noreply.github.com>

Fixed formatting

Update pass/fail to pass-fail for validation checks

Fixed formatting indentation

Changed pass/fail to pass-fail for GroupBy routing

Changed pass/fail to pass-fail for documentation

Fixed formatting for groupBy string cleansing

jcsmurph · 2023-12-11T16:04:52Z

Should be good to go now. It was a quick change to the standard out testing file.

jcsmurph · 2023-12-11T19:30:55Z

Looks like the new JUnit reporter needs to implement the groupBy print functions (either working or returning nil) or I could remove the groupBy print functions from the interface.

@kehoecj What do you think?

kehoecj · 2023-12-11T19:34:57Z

Looks like the new JUnit reporter needs to implement the groupBy print functions (either working or returning nil) or I could remove the groupBy print functions from the interface.

@kehoecj What do you think?

@jcsmurph Good question - I think Junit should be excluded from the groupby functionality. I don't see a lot of benefit to adding it since JUnit is really for processing. JUnit also has a pretty strict structure so I'm not even sure how feasible grouping it would be @jd4235 WDTY?

jd4235 · 2023-12-11T19:39:35Z

Looks like the new JUnit reporter needs to implement the groupBy print functions (either working or returning nil) or I could remove the groupBy print functions from the interface.
@kehoecj What do you think?

@jcsmurph Good question - I think Junit should be excluded from the groupby functionality. I don't see a lot of benefit to adding it since JUnit is really for processing. JUnit also has a pretty strict structure so I'm not even sure how feasible grouping it would be @jd4235 WDTY?

I agree that JUnit should be excluded from the grouping.

jcsmurph · 2023-12-11T20:02:41Z

I agree that JUnit should be excluded from the grouping.

Sounds good. The options I am seeing are:

Remove the groupBy functions from the interface. Refactor the functions.
Implement the groupBy functions for JUnit but return nil/an error saying it is not supported.
A third option I have not considered.

I should probably add a validation check for the flags if a user passes JUnit for the reporter, they cannot add a groupBy. That way the user gets the error immediately rather than after the validator has ran.

kehoecj · 2023-12-11T20:18:09Z

I agree that JUnit should be excluded from the grouping.

Sounds good. The options I am seeing are:

Remove the groupBy functions from the interface. Refactor the functions.

Implement the groupBy functions for JUnit but return nil/an error saying it is not supported.

A third option I have not considered.

I should probably add a validation check for the flags if a user passes JUnit for the reporter, they cannot add a groupBy. That way the user gets the error immediately rather than after the validator has ran.

Option 1 seems the cleanest to me. I agree with the validation check if a user passes groupby and junit

Refactored the print groupBy functions to be removed from the Reporter Interface

jcsmurph · 2023-12-11T21:38:09Z

Changes have been made and testing looks good on my end.

I am noticing now with the support for case mixed filetypes, they get sorted differently depending on the case. E.g. Csv is sorted differently than csv, CsV, etc. Did you want me to take a look into changing that as part of this PR?

Edit:

I realized it was an easy change. They are all being sorted into the same group now 😅

Updated grouping for filetypes to group mixed cases

Updated the group_output.go to align to gofmt standards

jcsmurph · 2023-12-12T04:28:42Z

I really need to figure out what is wrong with my local config and gofmt...

Sorry, you can try the workflow again. It should validate this time.

kehoecj · 2023-12-18T16:52:04Z

@jcsmurph I did a final functional test today running through several scenarios. There is currently an issue with --groupby directory functionality. For example running .\validator.exe -groupby directory .\test displays this output:

PS C:\> .\validator.exe -groupby directory .\test
/
    ✓ test\fixtures\exclude-file-types\excluded.json
    ✓ test\fixtures\exclude-file-types\not-excluded.toml
    ✓ test\fixtures\good.csv
    ✓ test\fixtures\good.hcl

This was definitely working before so it may be something that was introduced with one of the latest sets of changes. All my other functional tests passed so this should be the last issue before I'm ready to merge

jcsmurph · 2023-12-18T17:04:00Z

@kehoecj If I understand correctly, it is grouping the test\fixtures\ and test\fixtures\exclude-file-types\ directories together?

kehoecj · 2023-12-18T17:08:24Z

@kehoecj If I understand correctly, it is grouping the test\fixtures\ and test\fixtures\exclude-file-types\ directories together?

It's grouping everything under /. For example even running this commend PS C:\> .\validator.exe -groupby directory .\test C:\ProgramData\ / groups all the output from two totally separate places under /

jcsmurph · 2023-12-18T17:15:01Z

Got it. Seems to be Windows specific as I am not having the issue with my Linux setup but am with Windows. I will take a look.

Updated the directory groupby to check if the filepath is in Windows

jcsmurph · 2023-12-18T17:54:12Z

@kehoecj I made an update and it is working on my Windows machine now. Let me know if you see any other issues.

kehoecj · 2023-12-18T18:03:41Z

@kehoecj I made an update and it is working on my Windows machine now. Let me know if you see any other issues.

LGTM! Thanks for the quick turnaround

kehoecj · 2023-12-18T18:04:21Z

@jcsmurph One more issue - the change to fix windows dropped the coverage below the threshold.

Added a test for the GroupBy directory flag for Windows and other directory paths

jcsmurph · 2023-12-18T19:57:09Z

@kehoecj I added 2 tests for the directory GroupBy function to cover Windows file paths and others. That should hopefully get the test coverage back in the threshold.

kehoecj · 2023-12-18T20:04:36Z

@kehoecj I added 2 tests for the directory GroupBy function to cover Windows file paths and others. That should hopefully get the test coverage back in the threshold.

That did it - thanks!

kehoecj · 2023-12-18T20:05:03Z

Merged - thank you for the great work on this PR @jcsmurph !

jcsmurph · 2023-12-18T21:14:45Z

Awesome! I will keep an eye out for other issues I may want to tackle.

* Add GroupBy command Add -groupby command to group output by file type, pass/fail, and directory * Add multiple groupby commands and validation Add: Validation for the groupby command and return error if value passed is not a valid command Add: Allow for multiple values for the groupby command * Add Tests Add: Added tests for cli_test.go and validator_test.go * Reformat groupBy Reformatted the groupBy functions and groupBy validation * Add Test Files Added more test files for testing purposes * Update grouping order and update readme Updated the groupings to be done in reverse order. Updated the readme with information on the groupby command * Fix groupby directory Fixed the groupby directory function to group by the last directory in the path * Update Tests Updated CLI tests * Fix Comment Fixed the cleanString funciton comment * Update README.md Co-authored-by: Jamie Davidson <51518462+jd4235@users.noreply.github.com> * Update cli_test.go Fixed formatting * Update validator.go Update pass/fail to pass-fail for validation checks * Update validator_test.go Fixed formatting indentation * Update group_output.go Changed pass/fail to pass-fail for GroupBy routing * Update README.md Changed pass/fail to pass-fail for documentation * Update validator.go Fixed formatting for groupBy string cleansing * Update group_output.go Updated indentation * Update validator.go Update flag to pass-fail * Update cli_test.go Update pass/fail to pass-fail * Update group_output.go Update GroupByPassFail comment to be consistent with pass-fail * Formatting fix Update to formatting * Add Group output Organized output for when groupby flag is set * Implement Group Output standard out Implementation of standard out using single and double groupbys * Implement Single Groupby JSON Implement the JSON output for a single groupby call * Refactor GroupBy Refactored groupBy when 2 values are passed * Refactor GroupByTwo Refactored groupby when 2 values are passed * Update to Stdout Update to standard out print * Implement JSON single group Implement output for JSON when single group is passed * Implement JSON double group and tests Implemented the JSON output for when two groupbys are passed. Also added test cases in the CLI path * Update go fmt Updated running go fmt * Implement triple group output Implement output for when 3 groupbys are passed * Add Output Tests Added output tests for groupby outputs * Clean comments Cleaned up comments * Changed Summary Format Changed the summary per group to be aligned with the inner most group rather than the tests * Update README.md Co-authored-by: Clayton Kehoe <118750525+kehoecj@users.noreply.github.com> * Update cmd/validator/validator.go Update file type to filetype to avoid input confusion Co-authored-by: Clayton Kehoe <118750525+kehoecj@users.noreply.github.com> * Add Header Comments Added header comments to the stdout and json reporters * Fix Naming Conflict Fixed a naming conflict in the json reporter groupby when 3 groups are passed * Add Util function for Json Reporter Added a utility function to the JSON reporter to reduce duplicated code. * Rename Utility Function Renamed the utility function and added a header comment * Rename Utility Function Calls Renamed the utility function calls to the correct name * Update JSON Print Updated the JSON print to use the utility function * Update STD Reporter Test Updated the standard reporter tests to align with groupby function changes * Refactor Reporter Refactored the print groupBy functions to be removed from the Reporter Interface * Update GroupBy Filetype Updated grouping for filetypes to group mixed cases * GoFmt update Updated the group_output.go to align to gofmt standards * Update directory groupby Updated the directory groupby to check if the filepath is in Windows * Add GroupBy Directory test Added a test for the GroupBy directory flag for Windows and other directory paths --------- Co-authored-by: Jamie Davidson <51518462+jd4235@users.noreply.github.com> Co-authored-by: Clayton Kehoe <118750525+kehoecj@users.noreply.github.com>

jcsmurph added 9 commits November 19, 2023 12:58

Add GroupBy command

fb451a1

Add -groupby command to group output by file type, pass/fail, and directory

Add multiple groupby commands and validation

fdb4d5f

Add: Validation for the groupby command and return error if value passed is not a valid command Add: Allow for multiple values for the groupby command

Add Tests

4e048fd

Add: Added tests for cli_test.go and validator_test.go

Reformat groupBy

5683cb6

Reformatted the groupBy functions and groupBy validation

Add Test Files

a88b172

Added more test files for testing purposes

Update grouping order and update readme

7800a50

Updated the groupings to be done in reverse order. Updated the readme with information on the groupby command

Fix groupby directory

853efbd

Fixed the groupby directory function to group by the last directory in the path

Update Tests

bd74941

Updated CLI tests

Fix Comment

ca546e4

Fixed the cleanString funciton comment

jcsmurph changed the title ~~Add cli option to group reporter output #38~~ Add cli option to group reporter output (#38) Nov 20, 2023

jcsmurph changed the title ~~Add cli option to group reporter output (#38)~~ Add cli option to group reporter output. Closes #38 Nov 20, 2023

kehoecj requested review from jd4235 and kehoecj November 20, 2023 17:56

kehoecj added the OSS Community Contribution Contributions from the OSS Community label Nov 20, 2023

kehoecj reviewed Nov 20, 2023

View reviewed changes

jd4235 reviewed Nov 20, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

jd4235 reviewed Nov 20, 2023

View reviewed changes

pkg/cli/cli_test.go Outdated Show resolved Hide resolved

jd4235 reviewed Nov 20, 2023

View reviewed changes

pkg/cli/cli_test.go Outdated Show resolved Hide resolved

jd4235 reviewed Nov 20, 2023

View reviewed changes

pkg/cli/group_output.go Outdated Show resolved Hide resolved

jd4235 self-requested a review November 20, 2023 18:43

jcsmurph and others added 7 commits November 20, 2023 10:54

Update README.md

e61be66

Co-authored-by: Jamie Davidson <51518462+jd4235@users.noreply.github.com>

Update cli_test.go

1140303

Fixed formatting

Update validator.go

b333ed2

Update pass/fail to pass-fail for validation checks

Update validator_test.go

42e0c50

Fixed formatting indentation

Update group_output.go

1264e99

Changed pass/fail to pass-fail for GroupBy routing

Update README.md

49de011

Changed pass/fail to pass-fail for documentation

Update validator.go

89718be

Fixed formatting for groupBy string cleansing

Refactor Reporter

3bf77db

Refactored the print groupBy functions to be removed from the Reporter Interface

jcsmurph added 2 commits December 11, 2023 13:41

Update GroupBy Filetype

627b5c3

Updated grouping for filetypes to group mixed cases

GoFmt update

5262344

Updated the group_output.go to align to gofmt standards

kehoecj added this to the v1.6.0 milestone Dec 18, 2023

Update directory groupby

df6ac8c

Updated the directory groupby to check if the filepath is in Windows

kehoecj approved these changes Dec 18, 2023

View reviewed changes

Add GroupBy Directory test

99fb5b8

Added a test for the GroupBy directory flag for Windows and other directory paths

kehoecj merged commit 0efb7d0 into Boeing:main Dec 18, 2023
4 checks passed

jcsmurph deleted the Add-CLI-option-to-group-reporter-output-#38 branch December 18, 2023 21:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add cli option to group reporter output. Closes #38 #92

Add cli option to group reporter output. Closes #38 #92

jcsmurph commented Nov 20, 2023

jcsmurph commented Nov 20, 2023

kehoecj left a comment

jcsmurph commented Nov 20, 2023

jd4235 commented Nov 20, 2023

jcsmurph commented Dec 11, 2023

jcsmurph commented Dec 11, 2023

kehoecj commented Dec 11, 2023

jd4235 commented Dec 11, 2023

jcsmurph commented Dec 11, 2023

kehoecj commented Dec 11, 2023

jcsmurph commented Dec 11, 2023 •

edited

Loading

jcsmurph commented Dec 12, 2023

kehoecj commented Dec 18, 2023 •

edited

Loading

jcsmurph commented Dec 18, 2023

kehoecj commented Dec 18, 2023 •

edited

Loading

jcsmurph commented Dec 18, 2023

jcsmurph commented Dec 18, 2023

kehoecj commented Dec 18, 2023

kehoecj commented Dec 18, 2023

jcsmurph commented Dec 18, 2023

kehoecj commented Dec 18, 2023

kehoecj commented Dec 18, 2023

jcsmurph commented Dec 18, 2023

Add cli option to group reporter output. Closes #38 #92

Add cli option to group reporter output. Closes #38 #92

Conversation

jcsmurph commented Nov 20, 2023

jcsmurph commented Nov 20, 2023

kehoecj left a comment

Choose a reason for hiding this comment

jcsmurph commented Nov 20, 2023

jd4235 commented Nov 20, 2023

jcsmurph commented Dec 11, 2023

jcsmurph commented Dec 11, 2023

kehoecj commented Dec 11, 2023

jd4235 commented Dec 11, 2023

jcsmurph commented Dec 11, 2023

kehoecj commented Dec 11, 2023

jcsmurph commented Dec 11, 2023 • edited Loading

jcsmurph commented Dec 12, 2023

kehoecj commented Dec 18, 2023 • edited Loading

jcsmurph commented Dec 18, 2023

kehoecj commented Dec 18, 2023 • edited Loading

jcsmurph commented Dec 18, 2023

jcsmurph commented Dec 18, 2023

kehoecj commented Dec 18, 2023

kehoecj commented Dec 18, 2023

jcsmurph commented Dec 18, 2023

kehoecj commented Dec 18, 2023

kehoecj commented Dec 18, 2023

jcsmurph commented Dec 18, 2023

jcsmurph commented Dec 11, 2023 •

edited

Loading

kehoecj commented Dec 18, 2023 •

edited

Loading

kehoecj commented Dec 18, 2023 •

edited

Loading