From a729c0e2ae2d373b157c87d08f2c6c4785239322 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sat, 3 Dec 2022 13:06:00 +0100 Subject: [PATCH 1/2] tests/yanking: Extract `YankRequestHelper` trait to reduce code duplication --- src/tests/krate/yanking.rs | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/src/tests/krate/yanking.rs b/src/tests/krate/yanking.rs index 8ca2916b169..28543d81ed1 100644 --- a/src/tests/krate/yanking.rs +++ b/src/tests/krate/yanking.rs @@ -3,26 +3,15 @@ use crate::util::{RequestHelper, TestApp}; use crate::OkBool; use http::StatusCode; -impl crate::util::MockTokenUser { +trait YankRequestHelper { /// Yank the specified version of the specified crate and run all pending background jobs - fn yank(&self, krate_name: &str, version: &str) -> crate::util::Response { - let url = format!("/api/v1/crates/{krate_name}/{version}/yank"); - let response = self.delete(&url); - self.app().run_pending_background_jobs(); - response - } + fn yank(&self, krate_name: &str, version: &str) -> crate::util::Response; /// Unyank the specified version of the specified crate and run all pending background jobs - fn unyank(&self, krate_name: &str, version: &str) -> crate::util::Response { - let url = format!("/api/v1/crates/{krate_name}/{version}/unyank"); - let response = self.put(&url, &[]); - self.app().run_pending_background_jobs(); - response - } + fn unyank(&self, krate_name: &str, version: &str) -> crate::util::Response; } -impl crate::util::MockCookieUser { - /// Yank the specified version of the specified crate and run all pending background jobs +impl YankRequestHelper for T { fn yank(&self, krate_name: &str, version: &str) -> crate::util::Response { let url = format!("/api/v1/crates/{krate_name}/{version}/yank"); let response = self.delete(&url); @@ -30,7 +19,6 @@ impl crate::util::MockCookieUser { response } - /// Unyank the specified version of the specified crate and run all pending background jobs fn unyank(&self, krate_name: &str, version: &str) -> crate::util::Response { let url = format!("/api/v1/crates/{krate_name}/{version}/unyank"); let response = self.put(&url, &[]); From 6fa419c821286d3dbf848ed128fe04ea3fdd9d1b Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sat, 3 Dec 2022 13:14:17 +0100 Subject: [PATCH 2/2] tests/krate/yanking: Move route tests into `routes` submodules --- ...ank_unyank_unyank_records_an_audit_action} | 0 ..._yank_unyank_yank_records_an_audit_action} | 0 src/tests/krate/yanking.rs | 94 +------------------ src/tests/routes/crates/versions/mod.rs | 1 + .../routes/crates/versions/yank_unyank.rs | 93 ++++++++++++++++++ 5 files changed, 96 insertions(+), 92 deletions(-) rename src/tests/http-data/{krate_yanking_unyank_records_an_audit_action => routes_crates_versions_yank_unyank_unyank_records_an_audit_action} (100%) rename src/tests/http-data/{krate_yanking_yank_records_an_audit_action => routes_crates_versions_yank_unyank_yank_records_an_audit_action} (100%) create mode 100644 src/tests/routes/crates/versions/yank_unyank.rs diff --git a/src/tests/http-data/krate_yanking_unyank_records_an_audit_action b/src/tests/http-data/routes_crates_versions_yank_unyank_unyank_records_an_audit_action similarity index 100% rename from src/tests/http-data/krate_yanking_unyank_records_an_audit_action rename to src/tests/http-data/routes_crates_versions_yank_unyank_unyank_records_an_audit_action diff --git a/src/tests/http-data/krate_yanking_yank_records_an_audit_action b/src/tests/http-data/routes_crates_versions_yank_unyank_yank_records_an_audit_action similarity index 100% rename from src/tests/http-data/krate_yanking_yank_records_an_audit_action rename to src/tests/http-data/routes_crates_versions_yank_unyank_yank_records_an_audit_action diff --git a/src/tests/krate/yanking.rs b/src/tests/krate/yanking.rs index 28543d81ed1..9417bb19305 100644 --- a/src/tests/krate/yanking.rs +++ b/src/tests/krate/yanking.rs @@ -1,31 +1,6 @@ -use crate::builders::{CrateBuilder, PublishBuilder}; +use crate::builders::PublishBuilder; +use crate::routes::crates::versions::yank_unyank::YankRequestHelper; use crate::util::{RequestHelper, TestApp}; -use crate::OkBool; -use http::StatusCode; - -trait YankRequestHelper { - /// Yank the specified version of the specified crate and run all pending background jobs - fn yank(&self, krate_name: &str, version: &str) -> crate::util::Response; - - /// Unyank the specified version of the specified crate and run all pending background jobs - fn unyank(&self, krate_name: &str, version: &str) -> crate::util::Response; -} - -impl YankRequestHelper for T { - fn yank(&self, krate_name: &str, version: &str) -> crate::util::Response { - let url = format!("/api/v1/crates/{krate_name}/{version}/yank"); - let response = self.delete(&url); - self.app().run_pending_background_jobs(); - response - } - - fn unyank(&self, krate_name: &str, version: &str) -> crate::util::Response { - let url = format!("/api/v1/crates/{krate_name}/{version}/unyank"); - let response = self.put(&url, &[]); - self.app().run_pending_background_jobs(); - response - } -} #[test] #[allow(unknown_lints, clippy::bool_assert_comparison)] // for claim::assert_some_eq! with bool @@ -85,26 +60,6 @@ fn yank_works_as_intended() { assert!(!json.version.yanked); } -#[test] -fn yank_by_a_non_owner_fails() { - let (app, _, _, token) = TestApp::full().with_token(); - - let another_user = app.db_new_user("bar"); - let another_user = another_user.as_model(); - app.db(|conn| { - CrateBuilder::new("foo_not", another_user.id) - .version("1.0.0") - .expect_build(conn); - }); - - let response = token.yank("foo_not", "1.0.0"); - assert_eq!(response.status(), StatusCode::OK); - assert_eq!( - response.into_json(), - json!({ "errors": [{ "detail": "must already be an owner to yank or unyank" }] }) - ); -} - #[test] fn yank_max_version() { let (_, anon, _, token) = TestApp::full().with_token(); @@ -188,48 +143,3 @@ fn publish_after_yank_max_version() { let json = anon.show_crate("fyk_max"); assert_eq!(json.krate.max_version, "2.0.0"); } - -#[test] -fn yank_records_an_audit_action() { - let (_, anon, _, token) = TestApp::full().with_token(); - - // Upload a new crate, putting it in the git index - let crate_to_publish = PublishBuilder::new("fyk"); - token.publish_crate(crate_to_publish).good(); - - // Yank it - token.yank("fyk", "1.0.0").good(); - - // Make sure it has one publish and one yank audit action - let json = anon.show_version("fyk", "1.0.0"); - let actions = json.version.audit_actions; - - assert_eq!(actions.len(), 2); - let action = &actions[1]; - assert_eq!(action.action, "yank"); - assert_eq!(action.user.id, token.as_model().user_id); -} - -#[test] -fn unyank_records_an_audit_action() { - let (_, anon, _, token) = TestApp::full().with_token(); - - // Upload a new crate - let crate_to_publish = PublishBuilder::new("fyk"); - token.publish_crate(crate_to_publish).good(); - - // Yank version 1.0.0 - token.yank("fyk", "1.0.0").good(); - - // Unyank version 1.0.0 - token.unyank("fyk", "1.0.0").good(); - - // Make sure it has one publish, one yank, and one unyank audit action - let json = anon.show_version("fyk", "1.0.0"); - let actions = json.version.audit_actions; - - assert_eq!(actions.len(), 3); - let action = &actions[2]; - assert_eq!(action.action, "unyank"); - assert_eq!(action.user.id, token.as_model().user_id); -} diff --git a/src/tests/routes/crates/versions/mod.rs b/src/tests/routes/crates/versions/mod.rs index e390cf98a90..73deac5c613 100644 --- a/src/tests/routes/crates/versions/mod.rs +++ b/src/tests/routes/crates/versions/mod.rs @@ -2,3 +2,4 @@ mod authors; pub mod dependencies; pub mod download; mod read; +pub mod yank_unyank; diff --git a/src/tests/routes/crates/versions/yank_unyank.rs b/src/tests/routes/crates/versions/yank_unyank.rs new file mode 100644 index 00000000000..5a89bb9786a --- /dev/null +++ b/src/tests/routes/crates/versions/yank_unyank.rs @@ -0,0 +1,93 @@ +use crate::builders::{CrateBuilder, PublishBuilder}; +use crate::util::{RequestHelper, Response, TestApp}; +use crate::OkBool; +use http::StatusCode; + +pub trait YankRequestHelper { + /// Yank the specified version of the specified crate and run all pending background jobs + fn yank(&self, krate_name: &str, version: &str) -> Response; + + /// Unyank the specified version of the specified crate and run all pending background jobs + fn unyank(&self, krate_name: &str, version: &str) -> Response; +} + +impl YankRequestHelper for T { + fn yank(&self, krate_name: &str, version: &str) -> Response { + let url = format!("/api/v1/crates/{krate_name}/{version}/yank"); + let response = self.delete(&url); + self.app().run_pending_background_jobs(); + response + } + + fn unyank(&self, krate_name: &str, version: &str) -> Response { + let url = format!("/api/v1/crates/{krate_name}/{version}/unyank"); + let response = self.put(&url, &[]); + self.app().run_pending_background_jobs(); + response + } +} + +#[test] +fn yank_by_a_non_owner_fails() { + let (app, _, _, token) = TestApp::full().with_token(); + + let another_user = app.db_new_user("bar"); + let another_user = another_user.as_model(); + app.db(|conn| { + CrateBuilder::new("foo_not", another_user.id) + .version("1.0.0") + .expect_build(conn); + }); + + let response = token.yank("foo_not", "1.0.0"); + assert_eq!(response.status(), StatusCode::OK); + assert_eq!( + response.into_json(), + json!({ "errors": [{ "detail": "must already be an owner to yank or unyank" }] }) + ); +} + +#[test] +fn yank_records_an_audit_action() { + let (_, anon, _, token) = TestApp::full().with_token(); + + // Upload a new crate, putting it in the git index + let crate_to_publish = PublishBuilder::new("fyk"); + token.publish_crate(crate_to_publish).good(); + + // Yank it + token.yank("fyk", "1.0.0").good(); + + // Make sure it has one publish and one yank audit action + let json = anon.show_version("fyk", "1.0.0"); + let actions = json.version.audit_actions; + + assert_eq!(actions.len(), 2); + let action = &actions[1]; + assert_eq!(action.action, "yank"); + assert_eq!(action.user.id, token.as_model().user_id); +} + +#[test] +fn unyank_records_an_audit_action() { + let (_, anon, _, token) = TestApp::full().with_token(); + + // Upload a new crate + let crate_to_publish = PublishBuilder::new("fyk"); + token.publish_crate(crate_to_publish).good(); + + // Yank version 1.0.0 + token.yank("fyk", "1.0.0").good(); + + // Unyank version 1.0.0 + token.unyank("fyk", "1.0.0").good(); + + // Make sure it has one publish, one yank, and one unyank audit action + let json = anon.show_version("fyk", "1.0.0"); + let actions = json.version.audit_actions; + + assert_eq!(actions.len(), 3); + let action = &actions[2]; + assert_eq!(action.action, "unyank"); + assert_eq!(action.user.id, token.as_model().user_id); +}