From ab230cd52195db1ef24a80b2884e029d30f108f4 Mon Sep 17 00:00:00 2001 From: Mike Stemle Date: Mon, 11 Mar 2024 20:56:20 -0400 Subject: [PATCH 1/2] Fixed some `.gists()` functions, and wrote tests. - All three gist star-related functions suppressed errors, which has been fixed - `.gists().is_starred()` - 204 case covered - 404 case covered - 500 case covered - Now handles errors properly - `.gists().star()` - 204 case covered - 404 case covered - 500 case covered - Now handles errors properly - `.gists().unstar()` - 204 case covered - 304 case covered - 404 case covered - 500 case covered - Now handles errors properly - Removed an unused import from `src/api/checks.rs` --- src/api/checks.rs | 1 - src/api/gists.rs | 31 ++++-- tests/gists_test.rs | 225 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 249 insertions(+), 8 deletions(-) create mode 100644 tests/gists_test.rs diff --git a/src/api/checks.rs b/src/api/checks.rs index a722be65..79d95d2b 100644 --- a/src/api/checks.rs +++ b/src/api/checks.rs @@ -1,5 +1,4 @@ use chrono::{DateTime, Utc}; -use hyper::body::Body; use crate::models::checks::{AutoTriggerCheck, CheckSuite, CheckSuitePreferences}; use crate::models::{AppId, CheckRunId, CheckSuiteId}; diff --git a/src/api/gists.rs b/src/api/gists.rs index 9d44bd2c..0b6b8fed 100644 --- a/src/api/gists.rs +++ b/src/api/gists.rs @@ -263,7 +263,12 @@ impl<'octo> GistsHandler<'octo> { let gist_id = gist_id.as_ref(); let response = self.crab._get(format!("/gists/{gist_id}/star")).await?; // Gist API returns 204 (NO CONTENT) if a gist is starred - Ok(response.status() == StatusCode::NO_CONTENT) + + match response.status() { + StatusCode::NO_CONTENT => Ok(true), + StatusCode::NOT_FOUND => Ok(false), + _ => Err(crate::map_github_error(response).await.unwrap_err()), + } } /// Star the given gist. See [GitHub API Documentation][docs] more @@ -284,10 +289,16 @@ impl<'octo> GistsHandler<'octo> { let gist_id = gist_id.as_ref(); // PUT here returns an empty body, ignore it since it doesn't make // sense to deserialize it as JSON. - self.crab + let response = self + .crab ._put(format!("/gists/{gist_id}/star"), None::<&()>) - .await - .map(|_| ()) + .await?; + + if !response.status().is_success() { + return Err(crate::map_github_error(response).await.unwrap_err()); + } + + Ok(()) } /// Unstar the given gist. See [GitHub API Documentation][docs] more @@ -308,10 +319,16 @@ impl<'octo> GistsHandler<'octo> { let gist_id = gist_id.as_ref(); // DELETE here returns an empty body, ignore it since it doesn't make // sense to deserialize it as JSON. - self.crab + let response = self + .crab ._delete(format!("/gists/{gist_id}/star"), None::<&()>) - .await - .map(|_| ()) + .await?; + + if response.status() != StatusCode::NOT_MODIFIED && !response.status().is_success() { + return Err(crate::map_github_error(response).await.unwrap_err()); + } + + Ok(()) } /// Retrieve all the gists that forked the given `gist_id`. See diff --git a/tests/gists_test.rs b/tests/gists_test.rs new file mode 100644 index 00000000..c7cd8dfb --- /dev/null +++ b/tests/gists_test.rs @@ -0,0 +1,225 @@ +mod mock_error; + +use mock_error::setup_error_handler; +use octocrab::Octocrab; +use wiremock::{ + matchers::{method, path}, + Mock, MockServer, ResponseTemplate, +}; + +async fn setup_get_api(template: ResponseTemplate) -> MockServer { + let gist_id: &str = "12c55a94bd03166ff33ed0596263b4c6"; + + let mock_server = MockServer::start().await; + + Mock::given(method("GET")) + .and(path(format!("/gists/{gist_id}/star"))) + .respond_with(template.clone()) + .mount(&mock_server) + .await; + + setup_error_handler( + &mock_server, + &format!("GET on /gists/{gist_id}/star was not received"), + ) + .await; + mock_server +} + +async fn setup_delete_api(template: ResponseTemplate) -> MockServer { + let gist_id: &str = "12c55a94bd03166ff33ed0596263b4c6"; + + let mock_server = MockServer::start().await; + + Mock::given(method("DELETE")) + .and(path(format!("/gists/{gist_id}/star"))) + .respond_with(template.clone()) + .mount(&mock_server) + .await; + + setup_error_handler( + &mock_server, + &format!("DELETE on /gists/{gist_id}/star was not received"), + ) + .await; + mock_server +} + +async fn setup_put_api(template: ResponseTemplate) -> MockServer { + let gist_id: &str = "12c55a94bd03166ff33ed0596263b4c6"; + + let mock_server = MockServer::start().await; + + Mock::given(method("PUT")) + .and(path(format!("/gists/{gist_id}/star"))) + .respond_with(template.clone()) + .mount(&mock_server) + .await; + + setup_error_handler( + &mock_server, + &format!("PUT on /gists/{gist_id}/star was not received"), + ) + .await; + mock_server +} + +fn setup_octocrab(uri: &str) -> Octocrab { + Octocrab::builder().base_uri(uri).unwrap().build().unwrap() +} + +const GIST_ID: &str = "12c55a94bd03166ff33ed0596263b4c6"; + +#[tokio::test] +async fn test_get_gists_star_204() { + let template = ResponseTemplate::new(204); + let mock_server = setup_get_api(template).await; + let client = setup_octocrab(&mock_server.uri()); + + let result = client.gists().is_starred(GIST_ID.to_owned()).await; + + assert!( + result.is_ok(), + "expected successful result, got error: {:#?}", + result + ); + let result = result.unwrap(); + assert!(result, "expected the result to be true: {}", result); +} + +#[tokio::test] +async fn test_get_gists_star_404() { + let template = ResponseTemplate::new(404); + let mock_server = setup_get_api(template).await; + let client = setup_octocrab(&mock_server.uri()); + + let result = client.gists().is_starred(GIST_ID.to_owned()).await; + + assert!( + result.is_ok(), + "expected successful result, got error: {:#?}", + result + ); + let result = result.unwrap(); + assert!(!result, "expected the result to be false: {}", result); +} + +#[tokio::test] +async fn test_get_gists_star_500() { + let template = ResponseTemplate::new(500); + let mock_server = setup_get_api(template).await; + let client = setup_octocrab(&mock_server.uri()); + + let result = client.gists().is_starred(GIST_ID.to_owned()).await; + + assert!( + result.is_err(), + "expected error result, got success: {:#?}", + result + ); +} + +#[tokio::test] +async fn test_put_gists_star_204() { + let template = ResponseTemplate::new(204); + let mock_server = setup_put_api(template).await; + let client = setup_octocrab(&mock_server.uri()); + + let result = client.gists().star(GIST_ID.to_owned()).await; + + assert!( + result.is_ok(), + "expected successful result, got error: {:#?}", + result + ); +} + +#[tokio::test] +async fn test_put_gists_star_404() { + let template = ResponseTemplate::new(404); + let mock_server = setup_put_api(template).await; + let client = setup_octocrab(&mock_server.uri()); + + let result = client.gists().star(GIST_ID.to_owned()).await; + + assert!( + result.is_err(), + "expected error result, got success: {:#?}", + result + ); +} + +#[tokio::test] +async fn test_put_gists_star_500() { + let template = ResponseTemplate::new(500); + let mock_server = setup_put_api(template).await; + let client = setup_octocrab(&mock_server.uri()); + + let result = client.gists().star(GIST_ID.to_owned()).await; + + assert!( + result.is_err(), + "expected error result, got success: {:#?}", + result + ); +} + +#[tokio::test] +async fn test_delete_gists_star_204() { + let template = ResponseTemplate::new(204); + let mock_server = setup_delete_api(template).await; + let client = setup_octocrab(&mock_server.uri()); + + let result = client.gists().unstar(GIST_ID.to_owned()).await; + + assert!( + result.is_ok(), + "expected successful result, got error: {:#?}", + result + ); +} + +#[tokio::test] +async fn test_delete_gists_star_304() { + let template = ResponseTemplate::new(304); + let mock_server = setup_delete_api(template).await; + let client = setup_octocrab(&mock_server.uri()); + + let result = client.gists().unstar(GIST_ID.to_owned()).await; + + assert!( + result.is_ok(), + "expected successful result, got error: {:#?}", + result + ); +} + +#[tokio::test] +async fn test_delete_gists_star_404() { + let template = ResponseTemplate::new(404); + let mock_server = setup_delete_api(template).await; + let client = setup_octocrab(&mock_server.uri()); + + let result = client.gists().unstar(GIST_ID.to_owned()).await; + + assert!( + result.is_err(), + "expected error result, got success: {:#?}", + result + ); +} + +#[tokio::test] +async fn test_delete_gists_star_500() { + let template = ResponseTemplate::new(500); + let mock_server = setup_delete_api(template).await; + let client = setup_octocrab(&mock_server.uri()); + + let result = client.gists().unstar(GIST_ID.to_owned()).await; + + assert!( + result.is_err(), + "expected error result, got success: {:#?}", + result + ); +} From 0c81b2bf7a799add816e8497dd2d4fb9b09cb3c5 Mon Sep 17 00:00:00 2001 From: Mike Stemle Date: Sat, 16 Mar 2024 11:26:14 -0400 Subject: [PATCH 2/2] Delete a gist - Fixed the silent error cases - Added tests for 204, 304, 404, and 500 cases --- src/api/gists.rs | 12 ++++-- tests/gists_test.rs | 89 ++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 93 insertions(+), 8 deletions(-) diff --git a/src/api/gists.rs b/src/api/gists.rs index 0b6b8fed..1e8a5647 100644 --- a/src/api/gists.rs +++ b/src/api/gists.rs @@ -193,10 +193,16 @@ impl<'octo> GistsHandler<'octo> { /// ``` pub async fn delete(&self, gist_id: impl AsRef) -> Result<()> { let gist_id = gist_id.as_ref(); - self.crab + let response = self + .crab ._delete(format!("/gists/{gist_id}"), None::<&()>) - .await - .map(|_| ()) + .await?; + + if response.status() != StatusCode::NOT_MODIFIED && !response.status().is_success() { + return Err(crate::map_github_error(response).await.unwrap_err()); + } + + Ok(()) } /// Get a single gist revision. diff --git a/tests/gists_test.rs b/tests/gists_test.rs index c7cd8dfb..4102fe97 100644 --- a/tests/gists_test.rs +++ b/tests/gists_test.rs @@ -26,7 +26,7 @@ async fn setup_get_api(template: ResponseTemplate) -> MockServer { mock_server } -async fn setup_delete_api(template: ResponseTemplate) -> MockServer { +async fn setup_delete_star_api(template: ResponseTemplate) -> MockServer { let gist_id: &str = "12c55a94bd03166ff33ed0596263b4c6"; let mock_server = MockServer::start().await; @@ -45,6 +45,25 @@ async fn setup_delete_api(template: ResponseTemplate) -> MockServer { mock_server } +async fn setup_delete_gist_api(template: ResponseTemplate) -> MockServer { + let gist_id: &str = "12c55a94bd03166ff33ed0596263b4c6"; + + let mock_server = MockServer::start().await; + + Mock::given(method("DELETE")) + .and(path(format!("/gists/{gist_id}"))) + .respond_with(template.clone()) + .mount(&mock_server) + .await; + + setup_error_handler( + &mock_server, + &format!("DELETE on /gists/{gist_id} was not received"), + ) + .await; + mock_server +} + async fn setup_put_api(template: ResponseTemplate) -> MockServer { let gist_id: &str = "12c55a94bd03166ff33ed0596263b4c6"; @@ -167,7 +186,7 @@ async fn test_put_gists_star_500() { #[tokio::test] async fn test_delete_gists_star_204() { let template = ResponseTemplate::new(204); - let mock_server = setup_delete_api(template).await; + let mock_server = setup_delete_star_api(template).await; let client = setup_octocrab(&mock_server.uri()); let result = client.gists().unstar(GIST_ID.to_owned()).await; @@ -182,7 +201,7 @@ async fn test_delete_gists_star_204() { #[tokio::test] async fn test_delete_gists_star_304() { let template = ResponseTemplate::new(304); - let mock_server = setup_delete_api(template).await; + let mock_server = setup_delete_star_api(template).await; let client = setup_octocrab(&mock_server.uri()); let result = client.gists().unstar(GIST_ID.to_owned()).await; @@ -197,7 +216,7 @@ async fn test_delete_gists_star_304() { #[tokio::test] async fn test_delete_gists_star_404() { let template = ResponseTemplate::new(404); - let mock_server = setup_delete_api(template).await; + let mock_server = setup_delete_star_api(template).await; let client = setup_octocrab(&mock_server.uri()); let result = client.gists().unstar(GIST_ID.to_owned()).await; @@ -212,7 +231,7 @@ async fn test_delete_gists_star_404() { #[tokio::test] async fn test_delete_gists_star_500() { let template = ResponseTemplate::new(500); - let mock_server = setup_delete_api(template).await; + let mock_server = setup_delete_star_api(template).await; let client = setup_octocrab(&mock_server.uri()); let result = client.gists().unstar(GIST_ID.to_owned()).await; @@ -223,3 +242,63 @@ async fn test_delete_gists_star_500() { result ); } + +#[tokio::test] +async fn test_delete_gist_204() { + let template = ResponseTemplate::new(204); + let mock_server = setup_delete_gist_api(template).await; + let client = setup_octocrab(&mock_server.uri()); + + let result = client.gists().delete(GIST_ID.to_owned()).await; + + assert!( + result.is_ok(), + "expected successful result, got error: {:#?}", + result + ); +} + +#[tokio::test] +async fn test_delete_gist_304() { + let template = ResponseTemplate::new(304); + let mock_server = setup_delete_gist_api(template).await; + let client = setup_octocrab(&mock_server.uri()); + + let result = client.gists().delete(GIST_ID.to_owned()).await; + + assert!( + result.is_ok(), + "expected successful result, got error: {:#?}", + result + ); +} + +#[tokio::test] +async fn test_delete_gist_404() { + let template = ResponseTemplate::new(404); + let mock_server = setup_delete_gist_api(template).await; + let client = setup_octocrab(&mock_server.uri()); + + let result = client.gists().delete(GIST_ID.to_owned()).await; + + assert!( + result.is_err(), + "expected error result, got success: {:#?}", + result + ); +} + +#[tokio::test] +async fn test_delete_gist_500() { + let template = ResponseTemplate::new(500); + let mock_server = setup_delete_gist_api(template).await; + let client = setup_octocrab(&mock_server.uri()); + + let result = client.gists().delete(GIST_ID.to_owned()).await; + + assert!( + result.is_err(), + "expected error result, got success: {:#?}", + result + ); +}