From 6ec1176c0c81c3cf364f7d55ff47103ec589b495 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 1 Jun 2020 14:16:04 +0000 Subject: [PATCH 1/5] Add support for sorting according to severity --- misc.php | 27 +++++++++++++++++++++++++++ vip-go-ci.php | 14 ++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/misc.php b/misc.php index e0121f89c..0c983e932 100644 --- a/misc.php +++ b/misc.php @@ -1796,6 +1796,33 @@ function vipgoci_issues_filter_duplicate( $file_issues_arr ) { return $file_issues_arr_new; } +/* + * Sort issues to be submitted to GitHub according to + * severity of issues -- if configured to do so. + */ +function vipgoci_issues_sort_by_severity( + $options, + &$results +) { + + if ( true !== $options['issue-comments-sort'] ) { + return; + } + + vipgoci_log( + 'Sorting issues in results according to severity before submission', + array( + ) + ); + + + foreach( + $results['issues'] as + $pr_number => $pr_issues_comments + ) { + } +} + /* * Add pagebreak to a Markdown-style comment diff --git a/vip-go-ci.php b/vip-go-ci.php index cdd8c5e71..820d33c79 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -137,6 +137,7 @@ function vipgoci_run() { 'repo-name:', 'commit:', 'token:', + 'issue-comments-sort:', 'review-comments-max:', 'review-comments-total-max:', 'review-comments-ignore:', @@ -242,6 +243,8 @@ function vipgoci_run() { "\t" . '--repo-name=STRING Specify name of the repository' . PHP_EOL . "\t" . '--commit=STRING Specify the exact commit to scan (SHA)' . PHP_EOL . "\t" . '--token=STRING The access-token to use to communicate with GitHub' . PHP_EOL . + "\t" . '--issue-comments-sort=BOOL Sort issues found according to severity, from high ' . PHP_EOL . + "\t" . ' to low, before submitting to GitHub. Not sorted by default.' . PHP_EOL . "\t" . '--review-comments-max=NUMBER Maximum number of inline comments to submit' . PHP_EOL . "\t" . ' to GitHub in one review. If the number of ' . PHP_EOL . "\t" . ' comments exceed this number, additional reviews ' . PHP_EOL . @@ -727,6 +730,7 @@ function vipgoci_run() { vipgoci_option_bool_handle( $options, 'dismissed-reviews-repost-comments', 'true' ); + vipgoci_option_bool_handle( $options, 'issue-comments-sort', false ); if ( ( false === $options['lint'] ) && @@ -1600,6 +1604,16 @@ function vipgoci_run() { $prs_events_dismissed_by_team ); + /* + * Sort issues by severity level, so that + * highest severity is first. + */ + + vipgoci_results_sort_by_severity( + $options, + $results + ); + /* * Remove ignorable comments from $results. */ From 73fbd24209e0390199836b18522fa502e2ddea81 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 8 Jun 2020 17:57:53 +0000 Subject: [PATCH 2/5] Implement sorting, rename function --- misc.php | 53 +++++++++++++++++++++++++++++++++++++++++++++------ vip-go-ci.php | 6 +++--- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/misc.php b/misc.php index 0c983e932..e55a95343 100644 --- a/misc.php +++ b/misc.php @@ -1797,15 +1797,15 @@ function vipgoci_issues_filter_duplicate( $file_issues_arr ) { } /* - * Sort issues to be submitted to GitHub according to - * severity of issues -- if configured to do so. + * Sort results to be submitted to GitHub according to + * severity of issues -- if configured to do so: */ -function vipgoci_issues_sort_by_severity( +function vipgoci_results_sort_by_severity( $options, &$results ) { - if ( true !== $options['issue-comments-sort'] ) { + if ( true !== $options['results-comments-sort'] ) { return; } @@ -1817,9 +1817,50 @@ function vipgoci_issues_sort_by_severity( foreach( - $results['issues'] as - $pr_number => $pr_issues_comments + array_keys( + $results['issues'] + ) as $pr_number ) { + $current_pr_results = &$results['issues'][ $pr_number ]; + + /* + * Temporarily add severity + * column so we can sort using that. + */ + foreach( + array_keys( $current_pr_results ) as + $current_pr_result_item_key + ) { + $current_pr_results[ $current_pr_result_item_key ][ 'severity'] = + $current_pr_results[ $current_pr_result_item_key ]['issue']['severity']; + } + + /* + * Do the actual sorting. + */ + $severity_column = array_column( + $current_pr_results, + 'severity' + ); + + array_multisort( + $severity_column, + SORT_ASC, + $current_pr_results + ); + + /* + * Remove severity column + * afterwards. + */ + foreach( + array_keys( $current_pr_results ) as + $current_pr_result_item_key + ) { + unset( + $current_pr_results[ $current_pr_result_item_key ][ 'severity'] + ); + } } } diff --git a/vip-go-ci.php b/vip-go-ci.php index 820d33c79..49a2e3129 100755 --- a/vip-go-ci.php +++ b/vip-go-ci.php @@ -137,7 +137,7 @@ function vipgoci_run() { 'repo-name:', 'commit:', 'token:', - 'issue-comments-sort:', + 'results-comments-sort:', 'review-comments-max:', 'review-comments-total-max:', 'review-comments-ignore:', @@ -243,7 +243,7 @@ function vipgoci_run() { "\t" . '--repo-name=STRING Specify name of the repository' . PHP_EOL . "\t" . '--commit=STRING Specify the exact commit to scan (SHA)' . PHP_EOL . "\t" . '--token=STRING The access-token to use to communicate with GitHub' . PHP_EOL . - "\t" . '--issue-comments-sort=BOOL Sort issues found according to severity, from high ' . PHP_EOL . + "\t" . '--results-comments-sort=BOOL Sort issues found according to severity, from high ' . PHP_EOL . "\t" . ' to low, before submitting to GitHub. Not sorted by default.' . PHP_EOL . "\t" . '--review-comments-max=NUMBER Maximum number of inline comments to submit' . PHP_EOL . "\t" . ' to GitHub in one review. If the number of ' . PHP_EOL . @@ -730,7 +730,7 @@ function vipgoci_run() { vipgoci_option_bool_handle( $options, 'dismissed-reviews-repost-comments', 'true' ); - vipgoci_option_bool_handle( $options, 'issue-comments-sort', false ); + vipgoci_option_bool_handle( $options, 'results-comments-sort', false ); if ( ( false === $options['lint'] ) && From fa31fe651d67aeaa73752077c1949746522ef78f Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Mon, 8 Jun 2020 18:00:19 +0000 Subject: [PATCH 3/5] Sort DESC not ASC --- misc.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/misc.php b/misc.php index e55a95343..4264e0292 100644 --- a/misc.php +++ b/misc.php @@ -1845,7 +1845,7 @@ function vipgoci_results_sort_by_severity( array_multisort( $severity_column, - SORT_ASC, + SORT_DESC, $current_pr_results ); From cd7f4b312161854dff903738b9fac8560e3b9273 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 23 Jun 2020 13:15:52 +0000 Subject: [PATCH 4/5] Reverse results before posting to GitHub --- github-api.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/github-api.php b/github-api.php index 21ad54600..1915fd76e 100644 --- a/github-api.php +++ b/github-api.php @@ -2189,6 +2189,22 @@ function vipgoci_github_pr_review_submit( return; } + /* + * Reverse results before starting processing, + * so that results are shown in correct order + * after posting. + */ + + foreach ( + array_keys( + $results['issues'] + ) as $pr_number + ) { + $results['issues'][ $pr_number ] = array_reverse( + $results['issues'][ $pr_number ] + ); + } + foreach ( // The $results array is keyed by Pull-Request number array_keys( From cf98ebe924441750f541ad3b48cd951887b8bab9 Mon Sep 17 00:00:00 2001 From: Gudmundur Haraldsson Date: Tue, 23 Jun 2020 14:13:41 +0000 Subject: [PATCH 5/5] Adding test for vipgoci_results_sort_by_severity() --- tests/ResultsSortBySeverityTest.php | 324 ++++++++++++++++++++++++++++ 1 file changed, 324 insertions(+) create mode 100644 tests/ResultsSortBySeverityTest.php diff --git a/tests/ResultsSortBySeverityTest.php b/tests/ResultsSortBySeverityTest.php new file mode 100644 index 000000000..7c62b6ab6 --- /dev/null +++ b/tests/ResultsSortBySeverityTest.php @@ -0,0 +1,324 @@ +results = array( + 'issues' => array( + 24 => array( + array( + "type" => "phpcs", + "file_name" => "testfile1.php", + "file_line" => 100, + "issue" => array( + "message" => "Incorrect usage of function other_foo()", + "source" => "RandomStandard.OtherSniff.random_function", + "severity" => 100, + "fixable" => false, + "type" => "INFO", + "line" => 100, + "column" => 1, + "level" => "INFO" + ) + ), + array( + "type" => "phpcs", + "file_name" => "testfile1.php", + "file_line" => 3, + "issue" => array( + "message" => "Incorrect usage of function foo()", + "source" => "RandomStandard.RandomSniff.random_function", + "severity" => 3, + "fixable" => false, + "type" => "INFO", + "line" => 3, + "column" => 1, + "level" => "INFO" + ) + ), + array( + "type" => "phpcs", + "file_name" => "testfile1.php", + "file_line" => 2, + "issue" => array( + "message" => "Incorrect usage of function foo()", + "source" => "RandomStandard.RandomSniff.random_function", + "severity" => 40, + "fixable" => false, + "type" => "INFO", + "line" => 2, + "column" => 1, + "level" => "INFO" + ) + ), + array( + "type" => "phpcs", + "file_name" => "testfile2.php", + "file_line" => 100, + "issue" => array( + "message" => "Incorrect usage of function foo()", + "source" => "RandomStandard.RandomSniff.random_function", + "severity" => 101, + "fixable" => false, + "type" => "INFO", + "line" => 100, + "column" => 1, + "level" => "INFO" + ) + ), + array( + "type" => "phpcs", + "file_name" => "testfile2.php", + "file_line" => 100, + "issue" => array( + "message" => "Incorrect usage of function foo()", + "source" => "RandomStandard.RandomSniff.random_function", + "severity" => 37, + "fixable" => false, + "type" => "INFO", + "line" => 100, + "column" => 1, + "level" => "INFO" + ) + ), + ), + + 7 => array( + array( + "type" => "phpcs", + "file_name" => "testfile2.php", + "file_line" => 100, + "issue" => array( + "message" => "Incorrect usage of function foo()", + "source" => "RandomStandard.RandomSniff.random_function", + "severity" => 7, + "fixable" => false, + "type" => "INFO", + "line" => 100, + "column" => 1, + "level" => "INFO" + ) + ), + array( + "type" => "phpcs", + "file_name" => "testfile2.php", + "file_line" => 100, + "issue" => array( + "message" => "Incorrect usage of function testfoo()", + "source" => "RandomStandard.RandomSniff.test_function", + "severity" => 200, + "fixable" => false, + "type" => "INFO", + "line" => 100, + "column" => 1, + "level" => "INFO" + ) + ), + array( + "type" => "phpcs", + "file_name" => "testfile2.php", + "file_line" => 100, + "issue" => array( + "message" => "Incorrect usage of function myfoo()", + "source" => "RandomStandard.RandomSniff.myfoo_function", + "severity" => 377, + "fixable" => false, + "type" => "INFO", + "line" => 100, + "column" => 1, + "level" => "INFO" + ) + ), + ), + ) + ); + } + + protected function tearDown() { + $this->options = null; + $this->results = null; + } + + /** + * @covers ::vipgoci_results_sort_by_severity + */ + public function testSortingNotConfigured() { + $this->options['results-comments-sort'] = false; + $this->results_before = $this->results; + + vipgoci_unittests_output_suppress(); + + vipgoci_results_sort_by_severity( + $this->options, + $this->results + ); + + vipgoci_unittests_output_unsuppress(); + + $this->assertNotEmpty( + $this->results_before + ); + + // Not configured to sort, should remain unchanged + $this->assertEquals( + $this->results_before, + $this->results + ); + } + + /** + * @covers ::vipgoci_results_sort_by_severity + */ + public function testSortingCorrect1() { + $this->options['results-comments-sort'] = true; + + vipgoci_unittests_output_suppress(); + + vipgoci_results_sort_by_severity( + $this->options, + $this->results + ); + + vipgoci_unittests_output_unsuppress(); + + // Configured to sort, should be changed + $this->assertEquals( + array( + 'issues' => array( + 24 => array( + array( + "type" => "phpcs", + "file_name" => "testfile2.php", + "file_line" => 100, + "issue" => array( + "message" => "Incorrect usage of function foo()", + "source" => "RandomStandard.RandomSniff.random_function", + "severity" => 101, + "fixable" => false, + "type" => "INFO", + "line" => 100, + "column" => 1, + "level" => "INFO" + ) + ), + array( + "type" => "phpcs", + "file_name" => "testfile1.php", + "file_line" => 100, + "issue" => array( + "message" => "Incorrect usage of function other_foo()", + "source" => "RandomStandard.OtherSniff.random_function", + "severity" => 100, + "fixable" => false, + "type" => "INFO", + "line" => 100, + "column" => 1, + "level" => "INFO" + ) + ), + array( + "type" => "phpcs", + "file_name" => "testfile1.php", + "file_line" => 2, + "issue" => array( + "message" => "Incorrect usage of function foo()", + "source" => "RandomStandard.RandomSniff.random_function", + "severity" => 40, + "fixable" => false, + "type" => "INFO", + "line" => 2, + "column" => 1, + "level" => "INFO" + ) + ), + array( + "type" => "phpcs", + "file_name" => "testfile2.php", + "file_line" => 100, + "issue" => array( + "message" => "Incorrect usage of function foo()", + "source" => "RandomStandard.RandomSniff.random_function", + "severity" => 37, + "fixable" => false, + "type" => "INFO", + "line" => 100, + "column" => 1, + "level" => "INFO" + ) + ), + array( + "type" => "phpcs", + "file_name" => "testfile1.php", + "file_line" => 3, + "issue" => array( + "message" => "Incorrect usage of function foo()", + "source" => "RandomStandard.RandomSniff.random_function", + "severity" => 3, + "fixable" => false, + "type" => "INFO", + "line" => 3, + "column" => 1, + "level" => "INFO" + ) + ), + ), + + 7 => array( + array( + "type" => "phpcs", + "file_name" => "testfile2.php", + "file_line" => 100, + "issue" => array( + "message" => "Incorrect usage of function myfoo()", + "source" => "RandomStandard.RandomSniff.myfoo_function", + "severity" => 377, + "fixable" => false, + "type" => "INFO", + "line" => 100, + "column" => 1, + "level" => "INFO" + ) + ), + array( + "type" => "phpcs", + "file_name" => "testfile2.php", + "file_line" => 100, + "issue" => array( + "message" => "Incorrect usage of function testfoo()", + "source" => "RandomStandard.RandomSniff.test_function", + "severity" => 200, + "fixable" => false, + "type" => "INFO", + "line" => 100, + "column" => 1, + "level" => "INFO" + ) + ), + array( + "type" => "phpcs", + "file_name" => "testfile2.php", + "file_line" => 100, + "issue" => array( + "message" => "Incorrect usage of function foo()", + "source" => "RandomStandard.RandomSniff.random_function", + "severity" => 7, + "fixable" => false, + "type" => "INFO", + "line" => 100, + "column" => 1, + "level" => "INFO" + ) + ), + ), + ) + ), + + $this->results + ); + } + + +}