add tests; review error handling

Author: 2bndy5

cpp-linter/src/clang_tools/clang_format.rs

@@ -160,17 +160,13 @@ pub fn run_clang_format(
         .output()
         .with_context(|| format!("Failed to get replacements from clang-format: {file_name}"))?;
     if !output.stderr.is_empty() || !output.status.success() {
-        if let Ok(stderr) = String::from_utf8(output.stderr) {
-            logs.push((
-                log::Level::Debug,
-                format!("clang-format raised the follow errors:\n{}", stderr),
-            ));
-        } else {
-            logs.push((
-                log::Level::Error,
-                "stderr from clang-format was not UTF-8 encoded".to_string(),
-            ));
-        }
+        logs.push((
+            log::Level::Debug,
+            format!(
+                "clang-format raised the follow errors:\n{}",
+                String::from_utf8_lossy(&output.stderr)
+            ),
+        ));
     }
     if output.stdout.is_empty() {
         return Ok(logs);

cpp-linter/src/clang_tools/clang_tidy.rs

@@ -304,21 +304,17 @@ pub fn run_clang_tidy(
         log::Level::Debug,
         format!(
             "Output from clang-tidy:\n{}",
-            String::from_utf8(output.stdout.to_vec()).unwrap()
+            String::from_utf8_lossy(&output.stdout)
         ),
     ));
     if !output.stderr.is_empty() {
-        if let Ok(stderr) = String::from_utf8(output.stderr) {
-            logs.push((
-                log::Level::Debug,
-                format!("clang-tidy made the following summary:\n{}", stderr),
-            ));
-        } else {
-            logs.push((
-                log::Level::Error,
-                "clang-tidy stderr is not UTF-8 encoded".to_string(),
-            ));
-        }
+        logs.push((
+            log::Level::Debug,
+            format!(
+                "clang-tidy made the following summary:\n{}",
+                String::from_utf8_lossy(&output.stderr)
+            ),
+        ));
     }
     file.tidy_advice = parse_tidy_output(&output.stdout, &clang_params.database_json)?;
     if clang_params.tidy_review {

cpp-linter/src/rest_api/github/mod.rs

@@ -292,31 +292,31 @@ mod test {
         default::Default,
         env,
         io::Read,
-        path::PathBuf,
+        path::{Path, PathBuf},
         sync::{Arc, Mutex},
     };
 
-    use chrono::Utc;
-    use mockito::{Matcher, Server};
     use regex::Regex;
-    use reqwest::{Method, Url};
     use tempfile::{tempdir, NamedTempFile};
 
     use super::GithubApiClient;
     use crate::{
         clang_tools::capture_clang_tools_output,
         cli::{ClangParams, FeedbackInput, LinesChangedOnly},
-        common_fs::FileObj,
+        common_fs::{FileFilter, FileObj},
+        logger,
         rest_api::{RestApiClient, USER_OUTREACH},
     };
 
     // ************************* tests for step-summary and output variables
 
-    async fn create_comment(tidy_checks: &str, style: &str) -> (String, String) {
+    async fn create_comment(tidy_checks: &str, style: &str, fail_gh_out: bool) -> (String, String) {
         let tmp_dir = tempdir().unwrap();
         let rest_api_client = GithubApiClient::new().unwrap();
+        logger::init().unwrap();
         if env::var("ACTIONS_STEP_DEBUG").is_ok_and(|var| var == "true") {
             assert!(rest_api_client.debug_enabled);
+            log::set_max_level(log::LevelFilter::Debug);
         }
         let mut files = vec![Arc::new(Mutex::new(FileObj::new(PathBuf::from(
             "tests/demo/demo.cpp",
@@ -342,7 +342,14 @@ mod test {
         let mut step_summary_path = NamedTempFile::new_in(tmp_dir.path()).unwrap();
         env::set_var("GITHUB_STEP_SUMMARY", step_summary_path.path());
         let mut gh_out_path = NamedTempFile::new_in(tmp_dir.path()).unwrap();
-        env::set_var("GITHUB_OUTPUT", gh_out_path.path());
+        env::set_var(
+            "GITHUB_OUTPUT",
+            if fail_gh_out {
+                Path::new("not-a-file.txt")
+            } else {
+                gh_out_path.path()
+            },
+        );
         rest_api_client
             .post_feedback(&files, feedback_inputs)
             .await
@@ -354,13 +361,15 @@ mod test {
         assert!(&step_summary_content.contains(USER_OUTREACH));
         let mut gh_out_content = String::new();
         gh_out_path.read_to_string(&mut gh_out_content).unwrap();
-        assert!(gh_out_content.starts_with("checks-failed="));
+        if !fail_gh_out {
+            assert!(gh_out_content.starts_with("checks-failed="));
+        }
         (step_summary_content, gh_out_content)
     }
 
     #[tokio::test]
     async fn check_comment_concerns() {
-        let (comment, gh_out) = create_comment("readability-*", "file").await;
+        let (comment, gh_out) = create_comment("readability-*", "file", false).await;
         assert!(&comment.contains(":warning:\nSome files did not pass the configured checks!\n"));
         let fmt_pattern = Regex::new(r"format-checks-failed=(\d+)\n").unwrap();
         let tidy_pattern = Regex::new(r"tidy-checks-failed=(\d+)\n").unwrap();
@@ -380,61 +389,31 @@ mod test {
     #[tokio::test]
     async fn check_comment_lgtm() {
         env::set_var("ACTIONS_STEP_DEBUG", "true");
-        let (comment, gh_out) = create_comment("-*", "").await;
+        let (comment, gh_out) = create_comment("-*", "", false).await;
         assert!(&comment.contains(":heavy_check_mark:\nNo problems need attention."));
         assert_eq!(
             &gh_out,
             "checks-failed=0\nformat-checks-failed=0\ntidy-checks-failed=0\n"
         );
     }
 
-    async fn simulate_rate_limit(secondary: bool) {
-        let mut server = Server::new_async().await;
-        let url = Url::parse(server.url().as_str()).unwrap();
-        env::set_var("GITHUB_API_URL", server.url());
-        let client = GithubApiClient::new().unwrap();
-        let reset_timestamp = (Utc::now().timestamp() + 60).to_string();
-        let mock = server
-            .mock("GET", "/")
-            .match_body(Matcher::Any)
-            .expect_at_least(1)
-            .expect_at_most(5)
-            .with_status(429)
-            .with_header(
-                &client.rate_limit_headers.remaining,
-                if secondary { "1" } else { "0" },
-            )
-            .with_header(&client.rate_limit_headers.reset, &reset_timestamp);
-        if secondary {
-            mock.with_header(&client.rate_limit_headers.retry, "0")
-                .create();
-        } else {
-            mock.create();
-        }
-        let request =
-            GithubApiClient::make_api_request(&client.client, url, Method::GET, None, None)
-                .unwrap();
-        GithubApiClient::send_api_request(
-            client.client.clone(),
-            request,
-            client.rate_limit_headers.clone(),
-            0,
-        )
-        .await
-        .unwrap();
-    }
-
     #[tokio::test]
-    #[ignore]
-    #[should_panic(expected = "REST API secondary rate limit exceeded")]
-    async fn secondary_rate_limit() {
-        simulate_rate_limit(true).await;
+    async fn fail_gh_output() {
+        env::set_var("ACTIONS_STEP_DEBUG", "true");
+        let (comment, gh_out) = create_comment("-*", "", true).await;
+        assert!(&comment.contains(":heavy_check_mark:\nNo problems need attention."));
+        assert_eq!(&gh_out, "");
     }
 
     #[tokio::test]
-    #[ignore]
-    #[should_panic(expected = "REST API rate limit exceeded!")]
-    async fn primary_rate_limit() {
-        simulate_rate_limit(false).await;
+    async fn fail_get_local_diff() {
+        env::set_var("CI", "false");
+        let tmp_dir = tempdir().unwrap();
+        env::set_current_dir(tmp_dir.path()).unwrap();
+        let rest_client = GithubApiClient::new().unwrap();
+        let files = rest_client
+            .get_list_of_changed_files(&FileFilter::new(&[], vec![]))
+            .await;
+        assert!(files.is_err())
     }
 }

cpp-linter/src/rest_api/github/specific_api.rs

@@ -30,9 +30,9 @@ impl GithubApiClient {
         let pull_request = {
             match event_name.as_str() {
                 "pull_request" => {
-                    let event_payload_path = env::var("GITHUB_EVENT_PATH").with_context(|| {
-                        "GITHUB_EVENT_NAME is set to 'pull_request', but GITHUB_EVENT_PATH is not set"
-                    })?;
+                    // GITHUB_*** env vars cannot be overwritten in CI runners on GitHub.
+                    let event_payload_path = env::var("GITHUB_EVENT_PATH")?;
+                    // event payload JSON file can be overwritten/removed in CI runners
                     let file_buf = &mut String::new();
                     OpenOptions::new()
                         .read(true)
@@ -51,16 +51,15 @@ impl GithubApiClient {
                 _ => None,
             }
         };
+        // GITHUB_*** env vars cannot be overwritten in CI runners on GitHub.
         let gh_api_url = env::var("GITHUB_API_URL").unwrap_or("https://api.github.com".to_string());
-        let api_url = Url::parse(gh_api_url.clone().as_str())
-            .with_context(|| format!("Failed to parse URL from GITHUB_API_URL: {}", gh_api_url))?;
+        let api_url = Url::parse(gh_api_url.as_str())?;
 
         Ok(GithubApiClient {
             client: Client::builder()
                 .default_headers(Self::make_headers()?)
                 .user_agent(USER_AGENT)
-                .build()
-                .with_context(|| "Failed to create a session client for REST API calls")?,
+                .build()?,
             pull_request,
             event_name,
             api_url,
@@ -87,6 +86,7 @@ impl GithubApiClient {
     /// Append step summary to CI workflow's summary page.
     pub fn post_step_summary(&self, comment: &String) {
         if let Ok(gh_out) = env::var("GITHUB_STEP_SUMMARY") {
+            // step summary MD file can be overwritten/removed in CI runners
             if let Ok(mut gh_out_file) = OpenOptions::new().append(true).open(gh_out) {
                 if let Err(e) = writeln!(gh_out_file, "\n{}\n", comment) {
                     log::error!("Could not write to GITHUB_STEP_SUMMARY file: {}", e);

cpp-linter/src/rest_api/mod.rs

@@ -129,10 +129,10 @@
                                     "Failed to parse i64 from remaining attempts about rate limit: {count}"
                                 );
                             }
-                        } else {
-                            log::debug!("Failed to extract remaining attempts about rate limit: {remaining:?}");
                         }
                     } else {
+                        // NOTE: I guess it is sometimes valid for a request to
+                        // not include remaining rate limit attempts
                         log::debug!("Response headers do not include remaining API usage count");
                     }
                     if requests_remaining.is_some_and(|v| v <= 0) {
@@ -145,20 +145,17 @@
                                             "REST API rate limit exceeded! Resets at {}",
                                             reset
                                         ));
-                                    } else {
-                                        log::debug!("Rate limit reset UTC timestamp is an invalid: {value}");
                                     }
                                 } else {
                                     log::debug!(
                                         "Failed to parse i64 from reset time about rate limit: {epoch}"
                                     );
                                 }
-                            } else {
-                                log::debug!("Failed to extract reset info about rate limit: {reset_value:?}");
                             }
                         } else {
                             log::debug!("Response headers does not include a reset timestamp");
                         }
+                        return Err(anyhow!("REST API rate limit exceeded!"));
                     }
 
                     // check if secondary rate limit is violated; backoff and try again.
@@ -175,9 +172,7 @@
                                     "Failed to parse u64 from retry interval about rate limit: {retry_str}"
                                 );
                             }
-                        } else {
-                            log::debug!("Failed to extract retry interval about rate limit: {retry_value:?}");
                         }
                         return Self::send_api_request(
                             client,
                             request,
@@ -296,8 +291,6 @@
                         }
                     }
                 }
-            } else {
-                log::debug!("Failed to convert header value of links to a str");
             }
         }
         None
@@ -309,7 +302,7 @@
                 log::error!("{}: {e:?}", context.to_owned());
                 if let Ok(text) = response.text().await {
                     log::error!("{text}");
                 }
             }
         }
     }
@@ -390,3 +383,265 @@
     comment.push_str(&tidy_comment);
     comment.push_str(&closer);
 }
+
+/// This module tests the silent errors' debug logs
+/// from `try_next_page()` and `send_api_request()` functions.
+#[cfg(test)]
+mod test {
+    use std::sync::{Arc, Mutex};
+
+    use anyhow::{anyhow, Result};
+    use chrono::Utc;
+    use mockito::{Matcher, Server};
+    use reqwest::{
+        header::{HeaderMap, HeaderValue},
+        Client,
+    };
+    use reqwest::{Method, Url};
+
+    use crate::{
+        cli::FeedbackInput,
+        common_fs::{FileFilter, FileObj},
+        logger,
+    };
+
+    use super::{RestApiClient, RestApiRateLimitHeaders};
+
+    /// A dummy struct to impl RestApiClient
+    #[derive(Default)]
+    struct TestClient {}
+
+    impl RestApiClient for TestClient {
+        fn set_exit_code(
+            &self,
+            _checks_failed: u64,
+            _format_checks_failed: Option<u64>,
+            _tidy_checks_failed: Option<u64>,
+        ) -> u64 {
+            0
+        }
+
+        fn make_headers() -> Result<HeaderMap<HeaderValue>> {
+            Err(anyhow!("Not implemented"))
+        }
+
+        async fn get_list_of_changed_files(
+            &self,
+            _file_filter: &FileFilter,
+        ) -> Result<Vec<FileObj>> {
+            Err(anyhow!("Not implemented"))
+        }
+
+        async fn get_changed_files_paginated(
+            &self,
+            _url: reqwest::Url,
+            _file_filter: &FileFilter,
+        ) -> Result<Vec<FileObj>> {
+            Err(anyhow!("Not implemented"))
+        }
+
+        async fn post_feedback(
+            &self,
+            _files: &[Arc<Mutex<FileObj>>],
+            _user_inputs: FeedbackInput,
+        ) -> Result<u64> {
+            Err(anyhow!("Not implemented"))
+        }
+    }
+
+    #[tokio::test]
+    async fn dummy_coverage() {
+        assert!(TestClient::make_headers().is_err());
+        let dummy = TestClient::default();
+        assert_eq!(dummy.set_exit_code(1, None, None), 0);
+        assert!(dummy
+            .get_list_of_changed_files(&FileFilter::new(&[], vec![]))
+            .await
+            .is_err());
+        assert!(dummy
+            .get_changed_files_paginated(
+                Url::parse("https://example.net").unwrap(),
+                &FileFilter::new(&[], vec![])
+            )
+            .await
+            .is_err());
+        assert!(dummy
+            .post_feedback(&[], FeedbackInput::default())
+            .await
+            .is_err())
+    }
+
+    // ************************************************* try_next_page() tests
+
+    #[test]
+    fn bad_link_header() {
+        let mut headers = HeaderMap::with_capacity(1);
+        assert!(headers
+            .insert("link", HeaderValue::from_str("; rel=\"next\"").unwrap())
+            .is_none());
+        logger::init().unwrap();
+        log::set_max_level(log::LevelFilter::Debug);
+        let result = TestClient::try_next_page(&headers);
+        assert!(result.is_none());
+    }
+
+    #[test]
+    fn bad_link_domain() {
+        let mut headers = HeaderMap::with_capacity(1);
+        assert!(headers
+            .insert(
+                "link",
+                HeaderValue::from_str("<not a domain>; rel=\"next\"").unwrap()
+            )
+            .is_none());
+        logger::init().unwrap();
+        log::set_max_level(log::LevelFilter::Debug);
+        let result = TestClient::try_next_page(&headers);
+        assert!(result.is_none());
+    }
+
+    // ************************************************* Rate Limit Tests
+
+    #[derive(Default)]
+    struct RateLimitTestParams {
+        secondary: bool,
+        has_remaining_count: bool,
+        bad_remaining_count: bool,
+        has_reset_timestamp: bool,
+        bad_reset_timestamp: bool,
+        has_retry_interval: bool,
+        bad_retry_interval: bool,
+    }
+
+    async fn simulate_rate_limit(test_params: &RateLimitTestParams) {
+        let rate_limit_headers = RestApiRateLimitHeaders {
+            reset: "reset".to_string(),
+            remaining: "remaining".to_string(),
+            retry: "retry".to_string(),
+        };
+        logger::init().unwrap();
+        log::set_max_level(log::LevelFilter::Debug);
+
+        let mut server = Server::new_async().await;
+        let client = Client::new();
+        let reset_timestamp = (Utc::now().timestamp() + 60).to_string();
+        let mut mock = server
+            .mock("GET", "/")
+            .match_body(Matcher::Any)
+            .expect_at_least(1)
+            .expect_at_most(5)
+            .with_status(429);
+        if test_params.has_remaining_count {
+            mock = mock.with_header(
+                &rate_limit_headers.remaining,
+                if test_params.secondary {
+                    "1"
+                } else if test_params.bad_remaining_count {
+                    "X"
+                } else {
+                    "0"
+                },
+            );
+        }
+        if test_params.has_reset_timestamp {
+            mock = mock.with_header(
+                &rate_limit_headers.reset,
+                if test_params.bad_reset_timestamp {
+                    "X"
+                } else {
+                    &reset_timestamp
+                },
+            );
+        }
+        if test_params.secondary && test_params.has_retry_interval {
+            mock.with_header(
+                &rate_limit_headers.retry,
+                if test_params.bad_retry_interval {
+                    "X"
+                } else {
+                    "0"
+                },
+            )
+            .create();
+        } else {
+            mock.create();
+        }
+        let request =
+            TestClient::make_api_request(&client, server.url(), Method::GET, None, None).unwrap();
+        TestClient::send_api_request(client.clone(), request, rate_limit_headers.clone(), 0)
+            .await
+            .unwrap();
+    }
+
+    #[tokio::test]
+    #[ignore]
+    #[should_panic(expected = "REST API secondary rate limit exceeded")]
+    async fn rate_limit_secondary() {
+        simulate_rate_limit(&RateLimitTestParams {
+            secondary: true,
+            has_retry_interval: true,
+            ..Default::default()
+        })
+        .await;
+    }
+
+    #[tokio::test]
+    #[ignore]
+    #[should_panic(expected = "REST API secondary rate limit exceeded")]
+    async fn rate_limit_bad_retry() {
+        simulate_rate_limit(&RateLimitTestParams {
+            secondary: true,
+            has_retry_interval: true,
+            bad_retry_interval: true,
+            ..Default::default()
+        })
+        .await;
+    }
+
+    #[tokio::test]
+    #[ignore]
+    #[should_panic(expected = "REST API rate limit exceeded!")]
+    async fn rate_limit_primary() {
+        simulate_rate_limit(&RateLimitTestParams {
+            has_remaining_count: true,
+            has_reset_timestamp: true,
+            ..Default::default()
+        })
+        .await;
+    }
+
+    #[tokio::test]
+    #[ignore]
+    #[should_panic(expected = "REST API rate limit exceeded!")]
+    async fn rate_limit_no_reset() {
+        simulate_rate_limit(&RateLimitTestParams {
+            has_remaining_count: true,
+            ..Default::default()
+        })
+        .await;
+    }
+
+    #[tokio::test]
+    #[ignore]
+    #[should_panic(expected = "REST API rate limit exceeded!")]
+    async fn rate_limit_bad_reset() {
+        simulate_rate_limit(&RateLimitTestParams {
+            has_remaining_count: true,
+            has_reset_timestamp: true,
+            bad_reset_timestamp: true,
+            ..Default::default()
+        })
+        .await;
+    }
+
+    #[tokio::test]
+    #[ignore]
+    async fn rate_limit_bad_count() {
+        simulate_rate_limit(&RateLimitTestParams {
+            has_remaining_count: true,
+            bad_remaining_count: true,
+            ..Default::default()
+        })
+        .await;
+    }
+}

cpp-linter/src/run.rs

@@ -146,7 +146,7 @@ mod test {
     use std::env;
 
     #[tokio::test]
-    async fn run() {
+    async fn normal() {
         env::remove_var("GITHUB_OUTPUT"); // avoid writing to GH_OUT in parallel-running tests
         let result = run_main(vec![
             "cpp-linter".to_string(),
@@ -161,14 +161,14 @@ mod test {
     }
 
     #[tokio::test]
-    async fn run_version_command() {
+    async fn version_command() {
         env::remove_var("GITHUB_OUTPUT"); // avoid writing to GH_OUT in parallel-running tests
         let result = run_main(vec!["cpp-linter".to_string(), "version".to_string()]).await;
         assert!(result.is_ok_and(|v| v == 0));
     }
 
     #[tokio::test]
-    async fn run_force_debug_output() {
+    async fn force_debug_output() {
         env::remove_var("GITHUB_OUTPUT"); // avoid writing to GH_OUT in parallel-running tests
         let result = run_main(vec![
             "cpp-linter".to_string(),
@@ -182,7 +182,7 @@ mod test {
     }
 
     #[tokio::test]
-    async fn run_bad_version_input() {
+    async fn bad_version_input() {
         env::remove_var("GITHUB_OUTPUT"); // avoid writing to GH_OUT in parallel-running tests
         let result = run_main(vec![
             "cpp-linter".to_string(),
@@ -193,4 +193,17 @@ mod test {
         .await;
         assert!(result.is_ok_and(|v| v == 1));
     }
+
+    #[tokio::test]
+    async fn pre_commit_env() {
+        env::remove_var("GITHUB_OUTPUT"); // avoid writing to GH_OUT in parallel-running tests
+        env::set_var("PRE_COMMIT", "1");
+        let result = run_main(vec![
+            "cpp-linter".to_string(),
+            "-l".to_string(),
+            "false".to_string(),
+        ])
+        .await;
+        assert!(result.is_ok_and(|v| v == 1));
+    }
 }

cpp-linter/tests/paginated_changed_files.rs

@@ -22,8 +22,9 @@ const SHA: &str = "DEADBEEF";
 const TOKEN: &str = "123456";
 const RESET_RATE_LIMIT_HEADER: &str = "x-ratelimit-reset";
 const REMAINING_RATE_LIMIT_HEADER: &str = "x-ratelimit-remaining";
+const MALFORMED_RESPONSE_PAYLOAD: &str = "{\"message\":\"Resource not accessible by integration\"}";
 
-async fn get_paginated_changes(lib_root: &Path, event_type: EventType) {
+async fn get_paginated_changes(lib_root: &Path, event_type: EventType, fail_serialization: bool) {
     env::set_var("GITHUB_REPOSITORY", REPO);
     env::set_var("GITHUB_SHA", SHA);
     env::set_var("GITHUB_TOKEN", TOKEN);
@@ -84,68 +85,81 @@ async fn get_paginated_changes(lib_root: &Path, event_type: EventType) {
     } else {
         format!("{diff_end_point}/files")
     };
-    for pg in 1..=2 {
+    let pg_count = if fail_serialization { 1 } else { 2 };
+    for pg in 1..=pg_count {
         let link = if pg == 1 {
             format!("<{}{pg_end_point}?page=2>; rel=\"next\"", server.url())
         } else {
             "".to_string()
         };
-        mocks.push(
-            server
-                .mock("GET", pg_end_point.as_str())
-                .match_header("Accept", "application/vnd.github.raw+json")
-                .match_header("Authorization", format!("token {TOKEN}").as_str())
-                .match_query(Matcher::UrlEncoded("page".to_string(), pg.to_string()))
-                .with_header(REMAINING_RATE_LIMIT_HEADER, "50")
-                .with_header(RESET_RATE_LIMIT_HEADER, reset_timestamp.as_str())
-                .with_body_from_file(format!(
-                    "{asset_path}/{}_files_pg{pg}.json",
-                    if event_type == EventType::Push {
-                        "push"
-                    } else {
-                        "pull_request"
-                    }
-                ))
-                .with_header("link", link.as_str())
-                .create(),
-        );
+        let mut mock = server
+            .mock("GET", pg_end_point.as_str())
+            .match_header("Accept", "application/vnd.github.raw+json")
+            .match_header("Authorization", format!("token {TOKEN}").as_str())
+            .match_query(Matcher::UrlEncoded("page".to_string(), pg.to_string()))
+            .with_header(REMAINING_RATE_LIMIT_HEADER, "50")
+            .with_header(RESET_RATE_LIMIT_HEADER, reset_timestamp.as_str())
+            .with_header("link", link.as_str());
+        if fail_serialization {
+            mock = mock.with_body(MALFORMED_RESPONSE_PAYLOAD);
+        } else {
+            mock = mock.with_body_from_file(format!(
+                "{asset_path}/{}_files_pg{pg}.json",
+                if event_type == EventType::Push {
+                    "push"
+                } else {
+                    "pull_request"
+                }
+            ));
+        }
+        mocks.push(mock.create());
     }
 
     let file_filter = FileFilter::new(&[], vec!["cpp".to_string(), "hpp".to_string()]);
-    let files = gh_client
-        .get_list_of_changed_files(&file_filter)
-        .await
-        .unwrap();
-    assert_eq!(files.len(), 2);
-    for file in files {
-        assert!(["src/demo.cpp", "src/demo.hpp"].contains(
-            &file
-                .name
-                .as_path()
-                .to_str()
-                .expect("Failed to get file name from path")
-        ));
+    let files = gh_client.get_list_of_changed_files(&file_filter).await;
+    if let Ok(files) = files {
+        // if !fail_serialization
+        assert_eq!(files.len(), 2);
+        for file in files {
+            assert!(["src/demo.cpp", "src/demo.hpp"].contains(
+                &file
+                    .name
+                    .as_path()
+                    .to_str()
+                    .expect("Failed to get file name from path")
+            ));
+        }
     }
     for mock in mocks {
         mock.assert();
     }
 }
 
-async fn test_get_changes(event_type: EventType) {
+async fn test_get_changes(event_type: EventType, fail_serialization: bool) {
     let tmp_dir = create_test_space(false);
     let lib_root = env::current_dir().unwrap();
     env::set_current_dir(tmp_dir.path()).unwrap();
-    get_paginated_changes(&lib_root, event_type).await;
+    get_paginated_changes(&lib_root, event_type, fail_serialization).await;
     env::set_current_dir(lib_root.as_path()).unwrap();
     drop(tmp_dir);
 }
 
 #[tokio::test]
 async fn get_push_files_paginated() {
-    test_get_changes(EventType::Push).await
+    test_get_changes(EventType::Push, false).await
 }
 
 #[tokio::test]
 async fn get_pr_files_paginated() {
-    test_get_changes(EventType::PullRequest(42)).await
+    test_get_changes(EventType::PullRequest(42), false).await
+}
+
+#[tokio::test]
+async fn fail_push_files_paginated() {
+    test_get_changes(EventType::Push, true).await
+}
+
+#[tokio::test]
+async fn fail_pr_files_paginated() {
+    test_get_changes(EventType::PullRequest(42), true).await
 }