Remove input attachment hash tracking in tests

README.md

@@ -88,12 +88,9 @@ To add charts that have just been added to the CSV file into the database, run [
 
 The project provides an always-growing automated test suite for it's core functionality. The command logic is written in terms of a generic `MessagingContext` trait, which allows running the commands in non-discord contexts. The technique employed is called "golden testing" (also known as "snapshot testing") — the output of each test is initially saved to disk (at [test/commands](./test/commands)). On subsequent runs, the output is compared to the existing files, with the test failing on mismatches. You can provide the `SHIMMERING_TEST_REGEN=1` environment variable to override the existing output (make sure the changes are intended).
 
-Each test saves it's output in a directory. The directory contains two kinds of files:
+Each test saves it's output in a directory. Each file tracks the contents of a single response the bot produced during testing. This file contains everything from whether the response was a reply or not, to every field of every embed, to the hash of every attachment.
 
-- `in_<i>.toml`: these files track the hash of an attachment the command "downloaded" during execution. The screenshots used for testing are not available in this repository (thousands of Arcaea screenshots are posted to the internet on a daily basis, although I do not want to risk any legal trouble, therefore you need to provide your own), hence these hashes ensure the provided screenshots have not changed.
+The screenshots used for testing are not available in this repository. Although thousands of Arcaea screenshots are posted to the internet on a daily basis, I do not want to risk any legal trouble. You need to therefore provide your own testing screenshots. The test suite expects the following files to be present in `test/screenshots`:
-- `out_<i>.toml`: each such file tracks the contents of a single response the bot produced during testing. This file contains everything from whether the response was a reply or not, to each field of each provided embed, to the hash of every attachment.
-
-You should only bother setting up a testing environment if you are planning to maintain your own fork of this repository. As mentioned above, you need to provide your own screenshots for testing. The test suite expects the following files to be present in `test/screenshots`❌
 
 | File                         | Description                                 |
 | ---------------------------- | ------------------------------------------- |
@@ -103,6 +100,8 @@ You should only bother setting up a testing environment if you are planning to m
 | `antithese_74_kerning.jpg`   | a `9_983_744` score on `Antithese [FTR]`    |
 | `genocider_24_kerning.jpg`   | a `9_724_775` score on `GENOCIDER [FTR]`    |
 
+The hashes of the output images can often depend on the jacket images the tests were run with. This means you will likely have to regenerate the output locally in order to test with your own custom jackets.
+
 ## Thanks
 
 Many thanks go to:

src/bin/cli/context.rs

@@ -77,10 +77,7 @@ impl MessageContext for CliContext {
 		NonZeroU64::new(666).unwrap()
 	}
 
-	async fn download(
+	async fn download(&self, attachment: &Self::Attachment) -> Result<Vec<u8>, Error> {
-		_mutex: &tokio::sync::Mutex<&mut Self>,
-		attachment: &Self::Attachment,
-	) -> Result<Vec<u8>, Error> {
 		let res = tokio::fs::read(attachment).await?;
 		Ok(res)
 	}

src/commands/discord.rs

@@ -5,7 +5,6 @@ use std::str::FromStr;
 use poise::serenity_prelude::futures::future::join_all;
 use poise::serenity_prelude::{CreateAttachment, CreateEmbed};
 use poise::CreateReply;
-use tokio::sync::Mutex;
 
 use crate::arcaea::play::Play;
 use crate::context::{Error, ErrorKind, TaggedError, UserContext};
@@ -35,23 +34,17 @@ pub trait MessageContext {
 	fn attachment_id(attachment: &Self::Attachment) -> NonZeroU64;
 
 	/// Downloads a single file.
-	///
+	async fn download(&self, attachment: &Self::Attachment) -> Result<Vec<u8>, Error>;
-	/// This takes a mutex, as downloads are often done in parallel.
-	async fn download(
-		mutex: &tokio::sync::Mutex<&mut Self>,
-		attachment: &Self::Attachment,
-	) -> Result<Vec<u8>, Error>;
 
 	/// Downloads every image
 	async fn download_images<'a>(
-		&mut self,
+		&self,
 		attachments: &'a [Self::Attachment],
 	) -> Result<Vec<(&'a Self::Attachment, Vec<u8>)>, Error> {
-		let mutex = &Mutex::new(self);
 		let download_tasks = attachments
 			.iter()
 			.filter(|file| Self::is_image(file))
-			.map(|file| async move { (file, Self::download(mutex, file).await) });
+			.map(|file| async move { (file, self.download(file).await) });
 
 		let downloaded = timed!("dowload_files", { join_all(download_tasks).await });
 		downloaded
@@ -118,10 +111,7 @@ impl<'a> MessageContext for poise::Context<'a, UserContext, Error> {
 		attachment.dimensions().is_some()
 	}
 
-	async fn download(
+	async fn download(&self, attachment: &Self::Attachment) -> Result<Vec<u8>, Error> {
-		_mutex: &tokio::sync::Mutex<&mut Self>,
-		attachment: &Self::Attachment,
-	) -> Result<Vec<u8>, Error> {
 		let res = poise::serenity_prelude::Attachment::download(attachment).await?;
 		Ok(res)
 	}
@@ -135,7 +125,7 @@ pub mod mock {
 		path::{Path, PathBuf},
 	};
 
-	use anyhow::{anyhow, Context};
+	use anyhow::Context;
 	use poise::serenity_prelude::CreateEmbed;
 	use serde::{Deserialize, Serialize};
 	use sha2::{Digest, Sha256};
@@ -205,15 +195,11 @@ pub mod mock {
 	/// A mock context usable for testing. Messages and attachments are
 	/// accumulated inside a vec, and can be used for golden testing
 	/// (see [MockContext::golden]).
-	///
-	/// Moreover, downloaded attachment hashes are tracked so changes
-	/// to the input files require the test data to be regenerated.
 	pub struct MockContext {
 		pub user_id: u64,
 		pub data: UserContext,
 
 		messages: Vec<ReplyEssence>,
-		downloaded_files: Vec<AttachmentEssence>,
 	}
 
 	impl MockContext {
@@ -222,7 +208,6 @@ pub mod mock {
 				data,
 				user_id: 666,
 				messages: vec![],
-				downloaded_files: vec![],
 			}
 		}
 
@@ -242,13 +227,8 @@ pub mod mock {
 
 			fs::create_dir_all(path)?;
 
-			for (i, attachment) in self.downloaded_files.iter().enumerate() {
-				let file = path.join(format!("in_{i}.toml"));
-				Self::golden_impl(&file, &attachment)?;
-			}
-
 			for (i, message) in self.messages.iter().enumerate() {
-				let file = path.join(format!("out_{i}.toml"));
+				let file = path.join(format!("{i}.toml"));
 				Self::golden_impl(&file, message)?;
 			}
 
@@ -315,24 +295,11 @@ pub mod mock {
 			NonZeroU64::new(666).unwrap()
 		}
 
-		async fn download(
+		async fn download(&self, attachment: &Self::Attachment) -> Result<Vec<u8>, Error> {
-			mutex: &tokio::sync::Mutex<&mut Self>,
-			attachment: &Self::Attachment,
-		) -> Result<Vec<u8>, Error> {
 			let res = tokio::fs::read(attachment)
 				.await
 				.with_context(|| format!("Could not download attachment {attachment:?}"))?;
 
-			let mut guard = mutex.lock().await;
-			guard.downloaded_files.push(AttachmentEssence::new(
-				attachment
-					.to_str()
-					.ok_or_else(|| anyhow!("Download path contains invalid unicode"))?
-					.to_owned(),
-				None,
-				&res,
-			));
-
 			Ok(res)
 		}
 		// }}}

test/commands/chart/best/pick_correct_score/in_0.toml

@@ -1,2 +0,0 @@
-filename = "test/screenshots/antithese_74_kerning.jpg"
-hash = "sha256_7558ffb7c59018215955c4317ab14cb80f1d3bdf132d388c14a030ca526e2586"

test/commands/chart/best/pick_correct_score/in_1.toml

@@ -1,2 +0,0 @@
-filename = "test/screenshots/fracture_ray_ex.jpg"
-hash = "sha256_6aa2242fd40f71851ce826340fac5b53dbf3829568cf86bf5ed0ecc05ab89d2b"

test/commands/chart/best/pick_correct_score/in_2.toml

@@ -1,2 +0,0 @@
-filename = "test/screenshots/fracture_ray_missed_ex.jpg"
-hash = "sha256_6d6c10e9f946feb37df5925da0a14c70a9f42b68bb24a562bc295d301c956103"

test/commands/score/delete/delete_multiple/in_0.toml

@@ -1,2 +0,0 @@
-filename = "test/screenshots/antithese_74_kerning.jpg"
-hash = "sha256_7558ffb7c59018215955c4317ab14cb80f1d3bdf132d388c14a030ca526e2586"

test/commands/score/delete/delete_multiple/in_1.toml

@@ -1,2 +0,0 @@
-filename = "test/screenshots/alter_ego.jpg"
-hash = "sha256_5b88f5fc95b3146a0b2ed5c50e87410e4b26bb97026c293978790bf1344e0812"

test/commands/score/delete/delete_multiple/in_2.toml

@@ -1,2 +0,0 @@
-filename = "test/screenshots/genocider_24_kerning.jpg"
-hash = "sha256_45556caf74d37b0aa818faed3d7efe4baf870e25005b80963dbe60b89638f511"

test/commands/score/delete/delete_twice/in_0.toml

@@ -1,2 +0,0 @@
-filename = "test/screenshots/alter_ego.jpg"
-hash = "sha256_5b88f5fc95b3146a0b2ed5c50e87410e4b26bb97026c293978790bf1344e0812"

test/commands/score/delete/no_show_after_delete/in_0.toml

@@ -1,2 +0,0 @@
-filename = "test/screenshots/alter_ego.jpg"
-hash = "sha256_5b88f5fc95b3146a0b2ed5c50e87410e4b26bb97026c293978790bf1344e0812"

test/commands/score/magic/single_pic/in_0.toml

@@ -1,2 +0,0 @@
-filename = "test/screenshots/alter_ego.jpg"
-hash = "sha256_5b88f5fc95b3146a0b2ed5c50e87410e4b26bb97026c293978790bf1344e0812"

test/commands/score/magic/weird_kerning/in_0.toml

@@ -1,2 +0,0 @@
-filename = "test/screenshots/antithese_74_kerning.jpg"
-hash = "sha256_7558ffb7c59018215955c4317ab14cb80f1d3bdf132d388c14a030ca526e2586"

test/commands/score/magic/weird_kerning/in_1.toml

@@ -1,2 +0,0 @@
-filename = "test/screenshots/genocider_24_kerning.jpg"
-hash = "sha256_45556caf74d37b0aa818faed3d7efe4baf870e25005b80963dbe60b89638f511"

test/commands/score/show/agrees_with_magic/in_0.toml

@@ -1,2 +0,0 @@
-filename = "test/screenshots/antithese_74_kerning.jpg"
-hash = "sha256_7558ffb7c59018215955c4317ab14cb80f1d3bdf132d388c14a030ca526e2586"

test/commands/score/show/agrees_with_magic/in_1.toml

@@ -1,2 +0,0 @@
-filename = "test/screenshots/alter_ego.jpg"
-hash = "sha256_5b88f5fc95b3146a0b2ed5c50e87410e4b26bb97026c293978790bf1344e0812"

test/commands/score/show/agrees_with_magic/in_2.toml

@@ -1,2 +0,0 @@
-filename = "test/screenshots/genocider_24_kerning.jpg"
-hash = "sha256_45556caf74d37b0aa818faed3d7efe4baf870e25005b80963dbe60b89638f511"