tests: Use more doc comments to describe tests and test helpers (#5540)

Author: Turbo87

src/tests/owners.rs

@@ -170,8 +170,8 @@ fn create_and_add_owner(
     user
 }
 
-// Ensures that so long as at least one owner remains associated with the crate,
-// a user can still remove their own login as an owner
+/// Ensures that so long as at least one owner remains associated with the crate,
+/// a user can still remove their own login as an owner
 #[test]
 fn owners_can_remove_self() {
     let (app, _, user, token) = TestApp::init().with_token();
@@ -207,7 +207,7 @@ fn owners_can_remove_self() {
     );
 }
 
-// Verify consistency when adidng or removing multiple owners in a single request.
+/// Verify consistency when adidng or removing multiple owners in a single request.
 #[test]
 fn modify_multiple_owners() {
     let (app, _, user, token) = TestApp::init().with_token();
@@ -406,13 +406,12 @@ fn invite_with_existing_expired_invite() {
     assert_eq!(2, app.as_inner().emails.mails_in_memory().unwrap().len());
 }
 
-/*  Testing the crate ownership between two crates and one team.
-    Given two crates, one crate owned by both a team and a user,
-    one only owned by a user, check that the CrateList returned
-    for the user_id contains only the crates owned by that user,
-    and that the CrateList returned for the team_id contains
-    only crates owned by that team.
-*/
+/// Testing the crate ownership between two crates and one team.
+/// Given two crates, one crate owned by both a team and a user,
+/// one only owned by a user, check that the CrateList returned
+/// for the user_id contains only the crates owned by that user,
+/// and that the CrateList returned for the team_id contains
+/// only crates owned by that team.
 #[test]
 fn check_ownership_two_crates() {
     let (app, anon, user) = TestApp::init().with_user();
@@ -440,14 +439,13 @@ fn check_ownership_two_crates() {
     assert_eq!(json.crates[0].name, krate_owned_by_team.name);
 }
 
-/*  Given a crate owned by both a team and a user, check that the
-    JSON returned by the /owner_team route and /owner_user route
-    contains the correct kind of owner
-
-    Note that in this case function new_team must take a team name
-    of form github:org_name:team_name as that is the format
-    EncodableOwner::encodable is expecting
-*/
+/// Given a crate owned by both a team and a user, check that the
+/// JSON returned by the /owner_team route and /owner_user route
+/// contains the correct kind of owner
+///
+/// Note that in this case function new_team must take a team name
+/// of form github:org_name:team_name as that is the format
+/// EncodableOwner::encodable is expecting
 #[test]
 fn check_ownership_one_crate() {
     let (app, anon, user) = TestApp::init().with_user();
@@ -572,12 +570,11 @@ fn invitations_list_does_not_include_expired_invites_v1() {
     );
 }
 
-/*  Given a user inviting a different user to be a crate
-    owner, check that the user invited can accept their
-    invitation, the invitation will be deleted from
-    the invitations table, and a new crate owner will be
-    inserted into the table for the given crate.
-*/
+/// Given a user inviting a different user to be a crate
+/// owner, check that the user invited can accept their
+/// invitation, the invitation will be deleted from
+/// the invitations table, and a new crate owner will be
+/// inserted into the table for the given crate.
 #[test]
 fn test_accept_invitation() {
     let (app, anon, owner, owner_token) = TestApp::init().with_token();
@@ -600,11 +597,10 @@ fn test_accept_invitation() {
     assert_eq!(json.users.len(), 2);
 }
 
-/*  Given a user inviting a different user to be a crate
-    owner, check that the user invited can decline their
-    invitation and the invitation will be deleted from
-    the invitations table.
-*/
+/// Given a user inviting a different user to be a crate
+/// owner, check that the user invited can decline their
+/// invitation and the invitation will be deleted from
+/// the invitations table.
 #[test]
 fn test_decline_invitation() {
     let (app, anon, owner, owner_token) = TestApp::init().with_token();
@@ -652,8 +648,8 @@ fn test_accept_invitation_by_mail() {
     assert_eq!(json.users.len(), 2);
 }
 
-/// Hacky way to simulate the expiration of an ownership invitation. Instead of letting a month
-/// pass, the creation date of the invite is moved back a month.
+//// Hacky way to simulate the expiration of an ownership invitation. Instead of letting a month
+//// pass, the creation date of the invite is moved back a month.
 fn expire_invitation(app: &TestApp, crate_id: i32) {
     use cargo_registry::schema::crate_owner_invitations;
 

src/tests/record.rs

@@ -29,8 +29,8 @@ static IGNORED_HEADERS: &[&str] = &[
     "user-agent",
 ];
 
-// A "bomb" so when the test task exists we know when to shut down
-// the server and fail if the subtask failed.
+/// A "bomb" so when the test task exists we know when to shut down
+/// the server and fail if the subtask failed.
 pub struct Bomb {
     iorx: Sink,
     quittx: Option<oneshot::Sender<()>>,

src/tests/routes/crates/list.rs

@@ -554,13 +554,12 @@ fn max_stable_version() {
     assert_eq!(json.crates[0].max_stable_version, Some("1.0.0".to_string()));
 }
 
-/*  Given two crates, one with downloads less than 90 days ago, the
-    other with all downloads greater than 90 days ago, check that
-    the order returned is by recent downloads, descending. Check
-    also that recent download counts are returned in recent_downloads,
-    and total downloads counts are returned in downloads, and that
-    these numbers do not overlap.
-*/
+/// Given two crates, one with downloads less than 90 days ago, the
+/// other with all downloads greater than 90 days ago, check that
+/// the order returned is by recent downloads, descending. Check
+/// also that recent download counts are returned in recent_downloads,
+/// and total downloads counts are returned in downloads, and that
+/// these numbers do not overlap.
 #[test]
 fn test_recent_download_count() {
     let (app, anon, user) = TestApp::init().with_user();
@@ -595,10 +594,9 @@ fn test_recent_download_count() {
     assert_eq!(json.crates[1].downloads, 10);
 }
 
-/*  Given one crate with zero downloads, check that the crate
-   still shows up in index results, but that it displays 0
-   for both recent downloads and downloads.
-*/
+/// Given one crate with zero downloads, check that the crate
+/// still shows up in index results, but that it displays 0
+/// for both recent downloads and downloads.
 #[test]
 fn test_zero_downloads() {
     let (app, anon, user) = TestApp::init().with_user();
@@ -620,10 +618,9 @@ fn test_zero_downloads() {
     assert_eq!(json.crates[0].downloads, 0);
 }
 
-/*  Given two crates, one with more all-time downloads, the other with
-    more downloads in the past 90 days, check that the index page for
-    categories and keywords is sorted by recent downloads by default.
-*/
+/// Given two crates, one with more all-time downloads, the other with
+/// more downloads in the past 90 days, check that the index page for
+/// categories and keywords is sorted by recent downloads by default.
 #[test]
 fn test_default_sort_recent() {
     let (app, anon, user) = TestApp::init().with_user();

src/tests/team.rs

@@ -16,7 +16,7 @@ impl crate::util::MockAnonymousUser {
     }
 }
 
-// Test adding team without `github:`
+/// Test adding team without `github:`
 #[test]
 fn not_github() {
     let (app, _, user, token) = TestApp::init().with_token();
@@ -49,7 +49,7 @@ fn weird_name() {
     );
 }
 
-// Test adding team without second `:`
+/// Test adding team without second `:`
 #[test]
 fn one_colon() {
     let (app, _, user, token) = TestApp::init().with_token();
@@ -82,7 +82,7 @@ fn nonexistent_team() {
     );
 }
 
-// Test adding a renamed team
+/// Test adding a renamed team
 #[test]
 fn add_renamed_team() {
     let (app, anon) = TestApp::init().empty();
@@ -119,7 +119,7 @@ fn add_renamed_team() {
     assert_eq!(json.teams[0].login, "github:test-org:core");
 }
 
-// Test adding team names with mixed case, when on the team
+/// Test adding team names with mixed case, when on the team
 #[test]
 fn add_team_mixed_case() {
     let (app, anon) = TestApp::init().empty();
@@ -174,7 +174,7 @@ fn add_team_as_org_owner() {
     assert_eq!(json.teams[0].login, "github:test-org:core");
 }
 
-// Test adding team as owner when not on it
+/// Test adding team as owner when not on it
 #[test]
 fn add_team_as_non_member() {
     let (app, _) = TestApp::init().empty();
@@ -268,7 +268,7 @@ fn remove_team_as_team_owner() {
     );
 }
 
-// Test trying to publish a crate we don't own
+/// Test trying to publish a crate we don't own
 #[test]
 fn publish_not_owned() {
     let (app, _) = TestApp::full().empty();
@@ -319,7 +319,7 @@ fn publish_org_owner_owned() {
     );
 }
 
-// Test trying to publish a krate we do own (but only because of teams)
+/// Test trying to publish a krate we do own (but only because of teams)
 #[test]
 fn publish_owned() {
     let (app, _) = TestApp::full().empty();
@@ -340,7 +340,7 @@ fn publish_owned() {
     user_on_one_team.publish_crate(crate_to_publish).good();
 }
 
-// Test trying to change owners (when only on an owning team)
+/// Test trying to change owners (when only on an owning team)
 #[test]
 fn add_owners_as_org_owner() {
     let (app, _) = TestApp::init().empty();

src/tests/user.rs

@@ -374,16 +374,15 @@ fn updating_existing_user_doesnt_change_api_token() {
     assert_eq!("bar_token", user.gh_access_token);
 }
 
-/*  Given a GitHub user, check that if the user logs in,
-    updates their email, logs out, then logs back in, the
-    email they added to crates.io will not be overwritten
-    by the information sent by GitHub.
-
-    This bug is problematic if the user's email preferences
-    are set to private on GitHub, as GitHub will always
-    send none as the email and we will end up inadvertently
-    deleting their email when they sign back in.
-*/
+/// Given a GitHub user, check that if the user logs in,
+/// updates their email, logs out, then logs back in, the
+/// email they added to crates.io will not be overwritten
+/// by the information sent by GitHub.
+///
+/// This bug is problematic if the user's email preferences
+/// are set to private on GitHub, as GitHub will always
+/// send none as the email and we will end up inadvertently
+/// deleting their email when they sign back in.
 #[test]
 fn github_without_email_does_not_overwrite_email() {
     let (app, _) = TestApp::init().empty();
@@ -425,9 +424,8 @@ fn github_without_email_does_not_overwrite_email() {
     assert_eq!(json.user.email.unwrap(), "apricot@apricots.apricot");
 }
 
-/* Given a new user, test that if they sign in with one email, change their email on GitHub, then
-   sign in again, that the email in crates.io will remain set to the original email used on GitHub.
-*/
+/// Given a new user, test that if they sign in with one email, change their email on GitHub, then
+/// sign in again, that the email in crates.io will remain set to the original email used on GitHub.
 #[test]
 fn github_with_email_does_not_overwrite_email() {
     use cargo_registry::schema::emails;
@@ -461,10 +459,9 @@ fn github_with_email_does_not_overwrite_email() {
     assert_eq!(json.user.email, Some(original_email));
 }
 
-/*  Given a crates.io user, check that the user's email can be
-    updated in the database (PUT /user/:user_id), then check
-    that the updated email is sent back to the user (GET /me).
-*/
+/// Given a crates.io user, check that the user's email can be
+/// updated in the database (PUT /user/:user_id), then check
+/// that the updated email is sent back to the user (GET /me).
 #[test]
 fn test_email_get_and_put() {
     let (_app, _anon, user) = TestApp::init().with_user();
@@ -480,16 +477,15 @@ fn test_email_get_and_put() {
     assert!(json.user.email_verification_sent);
 }
 
-/*  Given a crates.io user, check to make sure that the user
-    cannot add to the database an empty string or null as
-    their email. If an attempt is made, update_user.rs will
-    return an error indicating that an empty email cannot be
-    added.
-
-    This is checked on the frontend already, but I'd like to
-    make sure that a user cannot get around that and delete
-    their email by adding an empty string.
-*/
+/// Given a crates.io user, check to make sure that the user
+/// cannot add to the database an empty string or null as
+/// their email. If an attempt is made, update_user.rs will
+/// return an error indicating that an empty email cannot be
+/// added.
+///
+/// This is checked on the frontend already, but I'd like to
+/// make sure that a user cannot get around that and delete
+/// their email by adding an empty string.
 #[test]
 fn test_empty_email_not_added() {
     let (_app, _anon, user) = TestApp::init().with_user();
@@ -510,12 +506,11 @@ fn test_empty_email_not_added() {
     );
 }
 
-/*  Check to make sure that neither other signed in users nor anonymous users can edit another
-    user's email address.
-
-    If an attempt is made, update_user.rs will return an error indicating that the current user
-    does not match the requested user.
-*/
+/// Check to make sure that neither other signed in users nor anonymous users can edit another
+/// user's email address.
+///
+/// If an attempt is made, update_user.rs will return an error indicating that the current user
+/// does not match the requested user.
 #[test]
 fn test_other_users_cannot_change_my_email() {
     let (app, anon, user) = TestApp::init().with_user();
@@ -543,12 +538,11 @@ fn test_other_users_cannot_change_my_email() {
     );
 }
 
-/* Given a new user, test that their email can be added
-   to the email table and a token for the email is generated
-   and added to the token table. When /confirm/:email_token is
-   requested, check that the response back is ok, and that
-   the email_verified field on user is now set to true.
-*/
+/// Given a new user, test that their email can be added
+/// to the email table and a token for the email is generated
+/// and added to the token table. When /confirm/:email_token is
+/// requested, check that the response back is ok, and that
+/// the email_verified field on user is now set to true.
 #[test]
 fn test_confirm_user_email() {
     use cargo_registry::schema::emails;
@@ -585,10 +579,9 @@ fn test_confirm_user_email() {
     assert!(json.user.email_verification_sent);
 }
 
-/* Given a user who existed before we added email confirmation,
-   test that `email_verification_sent` is false so that we don't
-   make the user think we've sent an email when we haven't.
-*/
+/// Given a user who existed before we added email confirmation,
+/// test that `email_verification_sent` is false so that we don't
+/// make the user think we've sent an email when we haven't.
 #[test]
 fn test_existing_user_email() {
     use cargo_registry::schema::emails;
@@ -638,9 +631,8 @@ fn test_user_owned_crates_doesnt_include_deleted_ownership() {
     assert_eq!(json.owned_crates.len(), 0);
 }
 
-/* A user should be able to update the email notifications for crates they own. Only the crates that
-   were sent in the request should be updated to the corresponding `email_notifications` value.
-*/
+/// A user should be able to update the email notifications for crates they own. Only the crates that
+/// were sent in the request should be updated to the corresponding `email_notifications` value.
 #[test]
 fn test_update_email_notifications() {
     let (app, _, user) = TestApp::init().with_user();
@@ -723,9 +715,8 @@ fn test_update_email_notifications() {
     })
 }
 
-/* A user should not be able to update the `email_notifications` value for a crate that is not
-   owned by them.
-*/
+/// A user should not be able to update the `email_notifications` value for a crate that is not
+/// owned by them.
 #[test]
 fn test_update_email_notifications_not_owned() {
     let (app, _, user) = TestApp::init().with_user();