diff --git a/defines.php b/defines.php index de83b054f..bbacf3dc9 100644 --- a/defines.php +++ b/defines.php @@ -154,3 +154,5 @@ => 'Maximum number of lines exceeded (%d)' ] ); + +define( 'VIPGOCI_VALIDATION_MAXIMUM_DETAIL_MSG', 'Note that the above file(s) were not analyzed due to their length.'); diff --git a/file-validation.php b/file-validation.php index b34e7aec6..c9e069709 100644 --- a/file-validation.php +++ b/file-validation.php @@ -68,7 +68,7 @@ function vipgoci_is_number_of_lines_valid( string $temp_file_name, string $file_ ); vipgoci_log( - 'Validation number of lines ', + 'Validating number of lines output', array( 'file_name' => $file_name, 'cmd' => $cmd, 'output' => $output ) ); diff --git a/github-api.php b/github-api.php index 92bdd91b5..be0f46001 100644 --- a/github-api.php +++ b/github-api.php @@ -2688,7 +2688,7 @@ function vipgoci_github_pr_review_submit( ( false === $github_errors ) && ( false === $github_warnings ) && ( false === $github_info ) && - empty( $results[ VIPGOCI_SKIPPED_FILES ][ $pr_number ] ) + empty( $results[ VIPGOCI_SKIPPED_FILES ][ $pr_number ]['issues'] ) ) { continue; } @@ -2807,7 +2807,11 @@ function vipgoci_github_pr_review_submit( /** * Format skipped files message if it validation has issues */ - if ( ! empty( $results[ VIPGOCI_SKIPPED_FILES ][ $pr_number ] ) ) { + if ( ! empty( $results[ VIPGOCI_SKIPPED_FILES ][ $pr_number ]['issues'] ) ) { + vipgoci_markdown_comment_add_pagebreak( + $github_postfields['body'] + ); + $github_postfields[ 'body' ] .= vipgoci_get_skipped_files_message( $results[ VIPGOCI_SKIPPED_FILES ][ $pr_number ], $pr_number diff --git a/main.php b/main.php index 8f25bdc25..dc20a7d56 100755 --- a/main.php +++ b/main.php @@ -2124,6 +2124,33 @@ function vipgoci_run() { ); } + /* + * Temporary: Log if any files were skipped. + */ + foreach ( $prs_implicated as $pr_item ) { + if ( ! empty( + $results[ + VIPGOCI_SKIPPED_FILES + ][ + $pr_item->number + ][ + 'issues' + ] + ) ) { + vipgoci_log( + 'Too large file(s) was/were detected during analysis: ' . + VIPGOCI_GITHUB_WEB_BASE_URL . '/' . $options['repo-owner'] . '/' . $options['repo-name'] . '/pull/' . $pr_item->number, + array( + 'repo_owner' => $options['repo-owner'], + 'repo_name' => $options['repo-name'], + 'pr_number' => $pr_item->number, + ), + 0, + true + ); + } + } + /* * Submit any remaining issues to GitHub */ diff --git a/misc.php b/misc.php index 0c1035878..893e3afd3 100644 --- a/misc.php +++ b/misc.php @@ -1235,6 +1235,13 @@ function vipgoci_markdown_comment_add_pagebreak( $comment_copy = rtrim( $comment ); $comment_copy = rtrim( $comment_copy, " \n\r" ); + /* + * If there is no comment, do not add pagebreak. + */ + if ( empty( $comment_copy ) ) { + return; + } + /* * Find the last pagebreak in the comment. */ diff --git a/options.php b/options.php index 12da0a119..9c8d8d25d 100644 --- a/options.php +++ b/options.php @@ -166,7 +166,7 @@ function vipgoci_options_read_repo_file( 'option_overwritable_conf' => $option_overwritable_conf, - 'repo_options_arr[' . $option_overwritable_name .' ]' + 'repo_options_arr[' . $option_overwritable_name . ']' => $repo_options_arr[ $option_overwritable_name ], 'repo_options_allowed' diff --git a/skip-file.php b/skip-file.php index e59c15c56..251352d6c 100644 --- a/skip-file.php +++ b/skip-file.php @@ -55,7 +55,7 @@ function vipgoci_set_prs_implicated_skipped_files( */ function vipgoci_get_skipped_files_message( array $skipped ): string { - $body = '****' . PHP_EOL . '**' . VIPGOCI_SKIPPED_FILES . '**' . PHP_EOL; + $body = PHP_EOL . '**' . VIPGOCI_SKIPPED_FILES . '**' . PHP_EOL . PHP_EOL; foreach ( $skipped[ 'issues' ] as $issue => $file ) { $body .= vipgoci_get_skipped_files_issue_message( $skipped[ 'issues' ][ $issue ], @@ -63,6 +63,8 @@ function vipgoci_get_skipped_files_message( array $skipped ): string ); } + $body .= PHP_EOL . PHP_EOL . VIPGOCI_VALIDATION_MAXIMUM_DETAIL_MSG; + return $body; } @@ -78,14 +80,14 @@ function vipgoci_get_skipped_files_issue_message( string $issue_type, int $max_lines = 15000 ): string { - $affected_files = implode( PHP_EOL . ' -', $affected_files ); + $affected_files = implode( PHP_EOL . ' - ', $affected_files ); $validation_message = sprintf( VIPGOCI_VALIDATION[ $issue_type ], $max_lines ); return sprintf( - '%s:%s -%s', + '%s:%s - %s', $validation_message, PHP_EOL, $affected_files diff --git a/svg-scan.php b/svg-scan.php index 0655c991a..eaf7a8326 100644 --- a/svg-scan.php +++ b/svg-scan.php @@ -227,6 +227,31 @@ function vipgoci_svg_scan_single_file( $file_contents ); + /* + * Skips the SVG scanning when the validation contains any issue + */ + if ( true === $options['skip-large-files'] ) { + $validation = vipgoci_validate( + $temp_file_name, + $file_name, + $options['commit'], + $options['skip-large-files-limit'] + ); + + if ( 0 !== $validation[ 'total' ] ) { + $skipped = array( + 'file_issues_arr_master' => array(), + 'file_issues_str' => null, + 'temp_file_name' => $temp_file_name, + 'validation' => $validation + ); + + unlink( $temp_file_name ); + + return $skipped; + } + } + /* * Use the svg-sanitizer's library scanner @@ -261,7 +286,7 @@ function vipgoci_svg_scan_single_file( 'file_issues_arr_master' => $results, 'file_issues_str' => null, 'temp_file_name' => $temp_file_name, - 'validation' => array( 'total' => 0 ) + 'validation' => array( 'total' => 0 ) ); } @@ -328,6 +353,7 @@ function( $issue_item ) { 'file_issues_arr_master' => $results, 'file_issues_str' => json_encode( $results ), 'temp_file_name' => $temp_file_name, + 'validation' => $validation ?? [], ); } diff --git a/tests/ApSvgFilesTest.php b/tests/ApSvgFilesTest.php index ec0f10690..6d614cac6 100644 --- a/tests/ApSvgFilesTest.php +++ b/tests/ApSvgFilesTest.php @@ -70,6 +70,10 @@ protected function setUp(): void { $this->options['branches-ignore'] = array(); $this->options['skip-draft-prs'] = false; + + $this->options['skip-large-files'] = false; + + $this->options['skip-large-files-limit'] = 15; } protected function tearDown(): void { diff --git a/tests/SvgScanScanCommitTest.php b/tests/SvgScanScanCommitTest.php new file mode 100644 index 000000000..861890a18 --- /dev/null +++ b/tests/SvgScanScanCommitTest.php @@ -0,0 +1,217 @@ + null, + 'commit-test-svg-scan-single-file-test-1' => null, // Re-use commit from SvgScanScanSingleFileTest + ); + + var $options_git_repo = array( + 'repo-owner' => null, + 'repo-name' => null, + 'git-path' => null, + 'github-repo-url' => null, + ); + + protected function setUp(): void { + vipgoci_unittests_get_config_values( + 'git', + $this->options_git_repo + ); + + vipgoci_unittests_get_config_values( + 'svg-scan', + $this->options_svg + ); + + $this->options = array_merge( + $this->options_git_repo, + $this->options_svg + ); + + $this->options['github-token'] = + vipgoci_unittests_get_config_value( + 'git-secrets', + 'github-token', + true // Fetch from secrets file + ); + + if ( empty( $this->options['github-token'] ) ) { + $this->options['github-token'] = ''; + } + + $this->options['token'] = + $this->options['github-token']; + + $this->options['branches-ignore'] = array(); + + $this->options['svg-checks'] = true; + + $this->options['lint-skip-folders'] = array(); + + $this->options['phpcs-skip-folders'] = array(); + + $this->options['skip-draft-prs'] = false; + + $this->options['phpcs-skip-scanning-via-labels-allowed'] = false; + + $this->options['skip-large-files'] = false; + + $this->options['skip-large-files-limit'] = 15; + } + + protected function tearDown(): void { + if ( false !== $this->options['local-git-repo'] ) { + vipgoci_unittests_remove_git_repo( + $this->options['local-git-repo'] + ); + } + + $this->options_svg = null; + $this->options_git_repo = null; + $this->options = null; + } + + /** + * Test SVG scanning of whole commit. + * + * @covers ::vipgoci_phpcs_scan_commit + */ + public function testDoScanTest1() { + $options_test = vipgoci_unittests_options_test( + $this->options, + array( 'github-token', 'token' ), + $this + ); + + if ( -1 === $options_test ) { + return; + } + + $this->options['commit'] = $this->options['commit-test-svg-scan-single-file-test-1']; + + $issues_submit = array(); + $issues_stats = array(); + $issues_skipped = array(); + + + $prs_implicated = $this->getPRsImplicated(); + + foreach( $prs_implicated as $pr_item ) { + $issues_stats[ + $pr_item->number + ][ + 'error' + ] = 0; + + $issues_skipped[ $pr_item->number ][ 'issues' ][ 'max-lines' ] = array(); + $issues_skipped[ $pr_item->number ][ 'issues' ][ 'total' ] = 0; + } + + vipgoci_unittests_output_suppress(); + + $this->options['local-git-repo'] = + vipgoci_unittests_setup_git_repo( + $this->options + ); + + if ( false === $this->options['local-git-repo'] ) { + $this->markTestSkipped( + 'Could not set up git repository: ' . + vipgoci_unittests_output_get() + ); + + return; + } + + vipgoci_phpcs_scan_commit( + $this->options, + $issues_submit, + $issues_stats, + $issues_skipped + ); + + vipgoci_unittests_output_unsuppress(); + + $this->assertSame( + array( + 5 => array( + array( + 'type' => 'phpcs', + 'file_name' => 'svg-file-with-issues-1.svg', + 'file_line' => 8, + 'issue' => array( + 'message' => "Suspicious attribute 'someotherfield2'", + 'line' => 8, + 'severity' => 5, + 'type' => 'ERROR', + 'source' => 'VipgociInternal.SVG.DisallowedTags', + 'level' => 'ERROR', + 'fixable' => false, + 'column' => 0, + ) + ), + + array( + 'type' => 'phpcs', + 'file_name' => 'svg-file-with-issues-1.svg', + 'file_line' => 5, + 'issue' => array( + 'message' => "Suspicious attribute 'myotherfield'", + 'line' => 5, + 'severity' => 5, + 'type' => 'ERROR', + 'source' => 'VipgociInternal.SVG.DisallowedTags', + 'level' => 'ERROR', + 'fixable' => false, + 'column' => 0, + ) + ) + ) + ), + + $issues_submit + ); + + $this->assertSame( + array( + 5 => array( + 'error' => 2, + ) + ), + $issues_stats + ); + } + + /** + * @return array|bool|mixed|null + */ + public function getPRsImplicated() { + $prs_implicated = vipgoci_github_prs_implicated( + $this->options['repo-owner'], + $this->options['repo-name'], + $this->options['commit'], + $this->options['github-token'], + $this->options['branches-ignore'] + ); + + return $prs_implicated; + } +} diff --git a/tests/SvgScanScanSingleFileTest.php b/tests/SvgScanScanSingleFileTest.php index 230db2cd5..d508199bc 100644 --- a/tests/SvgScanScanSingleFileTest.php +++ b/tests/SvgScanScanSingleFileTest.php @@ -44,6 +44,10 @@ protected function setUp(): void { $this->options['github-token']; $this->options['svg-checks'] = true; + + $this->options['skip-large-files'] = true; + + $this->options['skip-large-files-limit'] = 15000; } protected function tearDown(): void { @@ -139,6 +143,9 @@ public function testSvgScanSingleFileTest1() { 'file_issues_str' => '', 'temp_file_name' => $temp_file_name, + 'validation' => array( + 'total' => 0 + ), ); $expected_result['file_issues_str'] = json_encode( diff --git a/tests/VipgociSkipFileTest.php b/tests/VipgociSkipFileTest.php index 7c5fcbf2e..dc82840f2 100644 --- a/tests/VipgociSkipFileTest.php +++ b/tests/VipgociSkipFileTest.php @@ -179,11 +179,14 @@ public function testGetSkippedFilesMessage() { ); $skipped_files_message = vipgoci_get_skipped_files_message( $skipped ); - $expected_skipped_files_message = '**** + $expected_skipped_files_message = ' **skipped-files** + Maximum number of lines exceeded (15000): - -MyFailedClass.php - -MyFailedClass2.php'; + - MyFailedClass.php + - MyFailedClass2.php + +Note that the above file(s) were not analyzed due to their length.'; $this->assertSame( $expected_skipped_files_message, $skipped_files_message ); } @@ -200,8 +203,8 @@ public function testGetSkippedFilesIssueMessage() { ); $expected_skipped_files_issue_message = 'Maximum number of lines exceeded (15000): - -MyFailedClass.php - -MyFailedClass2.php'; + - MyFailedClass.php + - MyFailedClass2.php'; $this->assertSame( $expected_skipped_files_issue_message,