Skip to content

Commit 4958912

Browse files
authored
feat: add option for custom path to summary comment output (#388)
ref - cpp-linter/cpp-linter#205 - cpp-linter/cpp-linter#206 Allows exporting the step summary comment to a custom file path. Useful as a workaround for contexts with restricted permissions (eg private repos). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a new CLI option to save the generated summary to a file. * Summary files can use either an absolute path or a path relative to the repository root. * **Bug Fixes** * Summary content is now generated whenever file output is requested, even if step-based summary posting is off. * Improved error reporting for file and directory write failures. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 7ad731f commit 4958912

7 files changed

Lines changed: 101 additions & 5 deletions

File tree

cpp-linter/src/cli/mod.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,19 @@ pub struct FeedbackOptions {
494494
))]
495495
pub step_summary: bool,
496496

497+
/// Provide this option with a path to which a summary comment is written.
498+
///
499+
/// If the given path is relative, then it shall be relative to the given
500+
/// [`--repo-root`](#-r-repo-root) path.
501+
#[cfg_attr(feature = "bin", arg(
502+
short = 'o',
503+
long,
504+
value_name = "PATH",
505+
value_parser = value_parser!(PathBuf),
506+
help_heading = "Feedback options",
507+
))]
508+
pub summary_output_file: Option<PathBuf>,
509+
497510
/// Set this option to false to disable the use of
498511
/// file annotations as feedback.
499512
#[cfg_attr(feature = "bin", arg(

cpp-linter/src/cli/structs.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,9 @@ pub struct FeedbackInput {
299299
/// Whether to post a step summary comment.
300300
pub step_summary: bool,
301301

302+
/// An optional file path to which a summary comment is written.
303+
pub summary_output_file: Option<PathBuf>,
304+
302305
/// Whether to post file annotations.
303306
pub file_annotations: bool,
304307

@@ -330,6 +333,7 @@ impl From<&Cli> for FeedbackInput {
330333
pr_review: args.feedback_options.pr_review,
331334
passive_reviews: args.feedback_options.passive_reviews,
332335
repo_root: args.source_options.repo_root.clone(),
336+
summary_output_file: args.feedback_options.summary_output_file.clone(),
333337
}
334338
}
335339
}
@@ -346,6 +350,7 @@ impl Default for FeedbackInput {
346350
pr_review: false,
347351
passive_reviews: false,
348352
repo_root: PathBuf::from("."),
353+
summary_output_file: None,
349354
}
350355
}
351356
}

cpp-linter/src/common_fs.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,7 @@ impl FileObj {
391391
/// This function can error if the given path fails to be canonicalized.
392392
/// This may happen if the path does not exist or a non-final path
393393
/// component is not a directory.
394+
#[cfg(any(test, feature = "bin"))]
394395
pub(crate) fn mk_path_abs<P: AsRef<Path>>(path: P) -> Result<PathBuf, std::io::Error> {
395396
let abs_path = path.as_ref().canonicalize()?;
396397
let abs_path_str = abs_path.to_string_lossy();

cpp-linter/src/error.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,27 @@ pub enum ClientError {
4949
/// Error to propagate a [`FileObjError`] encountered during file processing.
5050
#[error(transparent)]
5151
FileObjError(#[from] FileObjError),
52+
53+
/// Error when failing to write a given summary output file.
54+
#[error("Failed to write summary comment to file '{file_path:?}': {source}")]
55+
SummaryOutputFileWriteFailed {
56+
/// The path to the summary output file that failed to be written.
57+
file_path: std::path::PathBuf,
58+
59+
/// The underlying error from trying to write the summary output file.
60+
#[source]
61+
source: std::io::Error,
62+
},
63+
64+
/// Error when failing to determine the parent directory of a given file path.
65+
#[error("Failed to create a parent directory for the path '{file_path:?}'")]
66+
MkDirFailed {
67+
/// The path to the summary output file that failed to be written.
68+
file_path: std::path::PathBuf,
69+
/// The underlying error from trying to create the parent directory.
70+
#[source]
71+
source: std::io::Error,
72+
},
5273
}
5374

5475
/// Errors related to invoking clang tools and processing their output.

cpp-linter/src/rest_client.rs

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,15 +127,36 @@ impl RestClient {
127127
let annotations = Self::make_annotations(files, &feedback_inputs.style)?;
128128
self.client.write_file_annotations(&annotations)?;
129129
}
130-
if feedback_inputs.step_summary {
130+
if feedback_inputs.step_summary || feedback_inputs.summary_output_file.is_some() {
131131
let summary = Self::make_comment(
132132
files,
133133
format_checks_failed,
134134
tidy_checks_failed,
135135
&clang_versions,
136136
None,
137137
)?;
138-
self.client.append_step_summary(&summary)?;
138+
if feedback_inputs.step_summary {
139+
self.client.append_step_summary(&summary)?;
140+
}
141+
if let Some(summary_output_file) = feedback_inputs.summary_output_file {
142+
let output_file = if summary_output_file.is_absolute() {
143+
summary_output_file
144+
} else {
145+
feedback_inputs.repo_root.join(&summary_output_file)
146+
};
147+
if let Some(parent) = output_file.parent() {
148+
std::fs::create_dir_all(parent).map_err(|e| ClientError::MkDirFailed {
149+
file_path: output_file.clone(),
150+
source: e,
151+
})?;
152+
}
153+
std::fs::write(&output_file, &summary).map_err(|e| {
154+
ClientError::SummaryOutputFileWriteFailed {
155+
file_path: output_file,
156+
source: e,
157+
}
158+
})?;
159+
}
139160
comment = Some(summary);
140161
}
141162
let output_vars = [

cpp-linter/tests/comments.rs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
#![cfg(feature = "bin")]
22
use chrono::Utc;
3-
use cpp_linter::cli::ThreadComments;
3+
use cpp_linter::rest_client::USER_OUTREACH;
44
use cpp_linter::run::run_main;
5+
use cpp_linter::{cli::ThreadComments, rest_client::COMMENT_MARKER};
56
use git_bot_feedback::LinesChangedOnly;
67
use mockito::Matcher;
78
use std::{env, fmt::Display, fs, io::Write, path::Path};
@@ -19,6 +20,8 @@ const MOCK_ASSETS_PATH: &str = "tests/comment_test_assets/";
1920
const RESET_RATE_LIMIT_HEADER: &str = "x-ratelimit-reset";
2021
const REMAINING_RATE_LIMIT_HEADER: &str = "x-ratelimit-remaining";
2122

23+
const SUMMARY_OUT_FILE_NAME: &str = "summary_output.md";
24+
2225
#[derive(PartialEq, Clone, Copy, Debug)]
2326
enum EventType {
2427
Push,
@@ -44,6 +47,7 @@ struct TestParams {
4447
pub fail_dismissal: bool,
4548
pub fail_posting: bool,
4649
pub bad_existing_comments: bool,
50+
pub relative_summary_out_file: bool,
4751
}
4852

4953
impl Default for TestParams {
@@ -58,6 +62,7 @@ impl Default for TestParams {
5862
fail_dismissal: false,
5963
fail_posting: false,
6064
bad_existing_comments: false,
65+
relative_summary_out_file: false,
6166
}
6267
}
6368
}
@@ -242,6 +247,12 @@ async fn setup(lib_root: &Path, tmp_dir: &TempDir, test_params: &TestParams) {
242247
);
243248
}
244249

250+
let summary_out_file_path = if test_params.relative_summary_out_file {
251+
std::path::PathBuf::from(SUMMARY_OUT_FILE_NAME)
252+
} else {
253+
tmp_dir.path().join(SUMMARY_OUT_FILE_NAME)
254+
};
255+
245256
let mut args = vec![
246257
"cpp-linter".to_string(),
247258
"-v=debug".to_string(),
@@ -254,6 +265,10 @@ async fn setup(lib_root: &Path, tmp_dir: &TempDir, test_params: &TestParams) {
254265
"-p=build".to_string(),
255266
"-i=build/**".to_string(),
256267
format!("--repo-root={}", tmp_dir.path().to_str().unwrap()),
268+
format!(
269+
"--summary-output-file={}",
270+
summary_out_file_path.to_str().unwrap()
271+
),
257272
];
258273
if test_params.force_lgtm {
259274
args.push("-e=c".to_string());
@@ -297,6 +312,15 @@ async fn setup(lib_root: &Path, tmp_dir: &TempDir, test_params: &TestParams) {
297312
std::fs::read_to_string(patch_path).expect("Failed to read generated patch file.");
298313
println!("Generated patch content:\n{patch_content}");
299314
}
315+
316+
let summary_out_file_abs_path = if test_params.relative_summary_out_file {
317+
tmp_dir.path().join(SUMMARY_OUT_FILE_NAME)
318+
} else {
319+
summary_out_file_path.clone()
320+
};
321+
let summary_content = std::fs::read_to_string(&summary_out_file_abs_path).unwrap();
322+
assert!(summary_content.contains(COMMENT_MARKER));
323+
assert!(summary_content.contains(USER_OUTREACH));
300324
}
301325

302326
async fn test_comment(test_params: &TestParams) {
@@ -473,3 +497,12 @@ async fn bad_existing_comments() {
473497
})
474498
.await;
475499
}
500+
501+
#[tokio::test]
502+
async fn relative_summary_out_file() {
503+
test_comment(&TestParams {
504+
relative_summary_out_file: true,
505+
..Default::default()
506+
})
507+
.await;
508+
}

docs/cli.yml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ inputs:
3131
minimum-version: '1.6.1'
3232
step-summary:
3333
minimum-version: '1.6.0'
34+
summary-output-file:
35+
minimum-version: '1.13.0'
3436
file-annotations:
3537
minimum-version: '1.4.3'
3638
database:
@@ -47,8 +49,6 @@ inputs:
4749
passive-reviews:
4850
minimum-version: '1.10.0'
4951
required-permission: 'pull-requests: write #pull-request-reviews'
50-
jobs:
51-
minimum-version: '1.8.1'
5252
diff-base:
5353
minimum-version: '1.12.0'
5454
ignore-index:
@@ -60,3 +60,5 @@ outputs:
6060
minimum-version: '1.6.2'
6161
clang-format-checks-failed:
6262
minimum-version: '1.6.2'
63+
fix-patch-path:
64+
minimum-version: '2.0.0'

0 commit comments

Comments
 (0)