prescientmoon/shimmeringmoon
prescientmoon/shimmeringmoon
1
Fork 0
Code Activity

Add more tests (and fix the score delete command)

Browse source 
Additionally, add a cli analogue of `score magic` (the `analyse`
subcommand)
This commit is contained in:
prescientmoon 2024-09-17 02:00:36 +02:00
parent 4ed0cadeb8
commit 0e0043d2c1
Signed by: prescientmoon
SSH key fingerprint: SHA256:WFp/cO76nbarETAoQcQXuV+0h7XJsEsOCI0UsyPIy6U
14 changed files with 542 additions and 200 deletions
src/commands
discord.rsscore.rs

62
src/commands/discord.rs
View file

@ -1,8 +1,9 @@
use std::num::NonZeroU64;
use std::{num::NonZeroU64, str::FromStr};
use poise::serenity_prelude::{futures::future::join_all, CreateAttachment, CreateMessage};
use crate::{
arcaea::play::Play,
context::{Error, UserContext},
timed,
};
@ -13,6 +14,9 @@ pub trait MessageContext {
fn data(&self) -> &UserContext;
fn author_id(&self) -> u64;
/// Fetch info about a user given it's id.
async fn fetch_user(&self, discord_id: &str) -> Result<poise::serenity_prelude::User, Error>;
/// Reply to the current message
async fn reply(&mut self, text: &str) -> Result<(), Error>;
@ -23,6 +27,11 @@ pub trait MessageContext {
message: CreateMessage,
) -> Result<(), Error>;
/// Deliver a message
async fn send(&mut self, message: CreateMessage) -> Result<(), Error> {
self.send_files([], message).await
}
// {{{ Input attachments
type Attachment;
@ -36,7 +45,7 @@ pub trait MessageContext {
/// Downloads every image
async fn download_images<'a>(
&self,
attachments: &'a Vec<Self::Attachment>,
attachments: &'a [Self::Attachment],
) -> Result<Vec<(&'a Self::Attachment, Vec<u8>)>, Error> {
let download_tasks = attachments
.iter()
@ -64,6 +73,13 @@ impl<'a> MessageContext for poise::Context<'a, UserContext, Error> {
self.author().id.get()
}
async fn fetch_user(&self, discord_id: &str) -> Result<poise::serenity_prelude::User, Error> {
poise::serenity_prelude::UserId::from_str(discord_id)?
.to_user(self.http())
.await
.map_err(|e| e.into())
}
async fn reply(&mut self, text: &str) -> Result<(), Error> {
Self::reply(*self, text).await?;
Ok(())
@ -106,6 +122,9 @@ pub mod mock {
use super::*;
/// A mock context usable for testing. Messages and attachments are
/// accumulated inside a vec, and can be used for golden testing
/// (see [MockContext::golden])
pub struct MockContext {
pub user_id: u64,
pub data: UserContext,
@ -121,7 +140,16 @@ pub mod mock {
}
}
pub fn write_to(&self, path: &PathBuf) -> Result<(), Error> {
// {{{ golden
/// This function implements the logic for "golden testing". We essentially
/// make sure a command's output doesn't change, by writing it to disk,
/// and comparing new outputs to the "golden" copy.
///
/// 1. This will attempt to write the data to disk (at the given path)
/// 2. If the data already exists on disk, the two copies will be
/// compared. A panic will occur on disagreements.
/// 3. `SHIMMERING_TEST_REGEN=1` can be passed to overwrite disagreements.
pub fn golden(&self, path: &PathBuf) -> Result<(), Error> {
if env::var("SHIMMERING_TEST_REGEN").unwrap_or_default() == "1" {
fs::remove_dir_all(path)?;
}
@ -152,10 +180,21 @@ pub mod mock {
fs::write(&path, &attachment.data)?;
}
}
// Ensure there's no extra attachments on disk
let file_count = fs::read_dir(dir)?.count();
if file_count != attachments.len() + 1 {
panic!(
"Only {} attachments found instead of {}",
attachments.len(),
file_count - 1
);
}
}
Ok(())
}
// }}}
}
impl MessageContext for MockContext {
@ -163,6 +202,16 @@ pub mod mock {
self.user_id
}
async fn fetch_user(
&self,
discord_id: &str,
) -> Result<poise::serenity_prelude::User, Error> {
let mut user = poise::serenity_prelude::User::default();
user.id = poise::serenity_prelude::UserId::from_str(discord_id)?;
user.name = "testinguser".to_string();
Ok(user)
}
fn data(&self) -> &UserContext {
&self.data
}
@ -208,3 +257,10 @@ pub mod mock {
}
}
// }}}
// {{{ Helpers
#[inline]
#[allow(dead_code)] // Currently only used for testing
pub fn play_song_title<'a>(ctx: &'a impl MessageContext, play: &'a Play) -> Result<&'a str, Error> {
Ok(&ctx.data().song_cache.lookup_chart(play.chart_id)?.0.title)
}
// }}}

323
src/commands/score.rs
View file

@ -2,7 +2,7 @@ use crate::arcaea::play::{CreatePlay, Play};
use crate::arcaea::score::Score;
use crate::context::{Context, Error};
use crate::recognition::recognize::{ImageAnalyzer, ScoreKind};
use crate::user::{discord_id_to_discord_user, User};
use crate::user::User;
use crate::{get_user, timed};
use anyhow::anyhow;
use image::DynamicImage;
@ -25,9 +25,9 @@ pub async fn score(_ctx: Context<'_>) -> Result<(), Error> {
// }}}
// {{{ Score magic
// {{{ Implementation
async fn magic_impl<C: MessageContext>(
pub async fn magic_impl<C: MessageContext>(
ctx: &mut C,
files: Vec<C::Attachment>,
files: &[C::Attachment],
) -> Result<Vec<Play>, Error> {
let user = get_user!(ctx);
let files = ctx.download_images(&files).await?;
@ -44,36 +44,30 @@ async fn magic_impl<C: MessageContext>(
for (i, (attachment, bytes)) in files.into_iter().enumerate() {
// {{{ Preapare image
let mut image = timed!("decode image", { image::load_from_memory(&bytes)? });
let mut grayscale_image = timed!("grayscale image", {
DynamicImage::ImageLuma8(image.to_luma8())
});
// image = image.resize(1024, 1024, FilterType::Nearest);
let mut image = image::load_from_memory(&bytes)?;
let mut grayscale_image = DynamicImage::ImageLuma8(image.to_luma8());
// }}}
let result: Result<(), Error> = try {
// {{{ Detection
// edit_reply!(ctx, handle, "Image {}: reading kind", i + 1).await?;
let kind = timed!("read_score_kind", {
analyzer.read_score_kind(ctx.data(), &grayscale_image)?
});
// edit_reply!(ctx, handle, "Image {}: reading difficulty", i + 1).await?;
// Do not use `ocr_image` because this reads the colors
let difficulty = timed!("read_difficulty", {
analyzer.read_difficulty(ctx.data(), &image, &grayscale_image, kind)?
});
// edit_reply!(ctx, handle, "Image {}: reading jacket", i + 1).await?;
let (song, chart) = timed!("read_jacket", {
analyzer.read_jacket(ctx.data(), &mut image, kind, difficulty)?
});
let max_recall = match kind {
ScoreKind::ScoreScreen => {
// edit_reply!(ctx, handle, "Image {}: reading max recall", i + 1).await?;
Some(analyzer.read_max_recall(ctx.data(), &grayscale_image)?)
// NOTE: are we ok with discarding errors like that?
analyzer.read_max_recall(ctx.data(), &grayscale_image).ok()
}
ScoreKind::SongSelect => None,
};
@ -81,13 +75,11 @@ async fn magic_impl<C: MessageContext>(
grayscale_image.invert();
let note_distribution = match kind {
ScoreKind::ScoreScreen => {
// edit_reply!(ctx, handle, "Image {}: reading distribution", i + 1).await?;
Some(analyzer.read_distribution(ctx.data(), &grayscale_image)?)
}
ScoreKind::SongSelect => None,
};
// edit_reply!(ctx, handle, "Image {}: reading score", i + 1).await?;
let score = timed!("read_score", {
analyzer
.read_score(ctx.data(), Some(chart.note_count), &grayscale_image, kind)
@ -145,19 +137,17 @@ mod magic_tests {
use std::path::PathBuf;
use crate::{
arcaea::score::ScoringSystem, commands::discord::mock::MockContext, with_test_ctx,
arcaea::score::ScoringSystem,
commands::discord::{mock::MockContext, play_song_title},
with_test_ctx,
};
use super::*;
fn play_song_title<'a>(ctx: &'a MockContext, play: &'a Play) -> Result<&'a str, Error> {
Ok(&ctx.data().song_cache.lookup_chart(play.chart_id)?.0.title)
}
#[tokio::test]
async fn no_pics() -> Result<(), Error> {
with_test_ctx!("test/commands/score/magic/no_pics", async |ctx| {
magic_impl(ctx, vec![]).await?;
magic_impl(ctx, &[]).await?;
Ok(())
})
}
@ -167,11 +157,9 @@ mod magic_tests {
with_test_ctx!(
"test/commands/score/magic/single_pic",
async |ctx: &mut MockContext| {
let plays = magic_impl(
ctx,
vec![PathBuf::from_str("test/screenshots/alter_ego.jpg")?],
)
.await?;
let plays =
magic_impl(ctx, &[PathBuf::from_str("test/screenshots/alter_ego.jpg")?])
.await?;
assert_eq!(plays.len(), 1);
assert_eq!(plays[0].score(ScoringSystem::Standard).0, 9926250);
assert_eq!(play_song_title(ctx, &plays[0])?, "ALTER EGO");
@ -187,7 +175,7 @@ mod magic_tests {
async |ctx: &mut MockContext| {
let plays = magic_impl(
ctx,
vec![
&[
PathBuf::from_str("test/screenshots/antithese_74_kerning.jpg")?,
PathBuf::from_str("test/screenshots/genocider_24_kerning.jpg")?,
],
@ -206,74 +194,33 @@ mod magic_tests {
}
}
// }}}
// {{{ Discord wrapper
/// Identify scores from attached images.
#[poise::command(prefix_command, slash_command)]
pub async fn magic(
mut ctx: Context<'_>,
#[description = "Images containing scores"] files: Vec<serenity::Attachment>,
) -> Result<(), Error> {
magic_impl(&mut ctx, files).await?;
magic_impl(&mut ctx, &files).await?;
Ok(())
}
// }}}
// {{{ Score delete
/// Delete scores, given their IDs.
#[poise::command(prefix_command, slash_command)]
pub async fn delete(
mut ctx: Context<'_>,
#[description = "Id of score to delete"] ids: Vec<u32>,
) -> Result<(), Error> {
let user = get_user!(&mut ctx);
if ids.len() == 0 {
ctx.reply("Empty ID list provided").await?;
return Ok(());
}
let mut count = 0;
for id in ids {
let res = ctx
.data()
.db
.get()?
.prepare_cached("DELETE FROM plays WHERE id=? AND user_id=?")?
.execute((id, user.id))?;
if res == 0 {
ctx.reply(format!("No play with id {} found", id)).await?;
} else {
count += 1;
}
}
if count > 0 {
ctx.reply(format!("Deleted {} play(s) successfully!", count))
.await?;
}
Ok(())
}
// }}}
// {{{ Score show
/// Show scores given their ides
#[poise::command(prefix_command, slash_command)]
pub async fn show(
ctx: Context<'_>,
#[description = "Ids of score to show"] ids: Vec<u32>,
) -> Result<(), Error> {
// {{{ Implementation
pub async fn show_impl<C: MessageContext>(ctx: &mut C, ids: &[u32]) -> Result<Vec<Play>, Error> {
if ids.len() == 0 {
ctx.reply("Empty ID list provided").await?;
return Ok(());
return Ok(vec![]);
}
let mut embeds = Vec::with_capacity(ids.len());
let mut attachments = Vec::with_capacity(ids.len());
let mut plays = Vec::with_capacity(ids.len());
let conn = ctx.data().db.get()?;
for (i, id) in ids.iter().enumerate() {
let (song, chart, play, discord_id) = conn
let result = conn
.prepare_cached(
"
SELECT
@ -292,13 +239,24 @@ pub async fn show(
.query_and_then([id], |row| -> Result<_, Error> {
let (song, chart) = ctx.data().song_cache.lookup_chart(row.get("chart_id")?)?;
let play = Play::from_sql(chart, row)?;
let discord_id = row.get::<_, String>("discord_id")?;
Ok((song, chart, play, discord_id))
})?
.next()
.ok_or_else(|| anyhow!("Could not find play with id {}", id))??;
.next();
let author = discord_id_to_discord_user(&ctx, &discord_id).await?;
let (song, chart, play, discord_id) = match result {
None => {
ctx.send(
CreateMessage::new().content(format!("Could not find play with id {}", id)),
)
.await?;
continue;
}
Some(result) => result?,
};
let author = ctx.fetch_user(&discord_id).await?;
let user = User::by_id(ctx.data(), play.user_id)?;
let (embed, attachment) =
@ -306,12 +264,215 @@ pub async fn show(
embeds.push(embed);
attachments.extend(attachment);
plays.push(play);
}
ctx.channel_id()
.send_files(ctx.http(), attachments, CreateMessage::new().embeds(embeds))
.await?;
if embeds.len() > 0 {
ctx.send_files(attachments, CreateMessage::new().embeds(embeds))
.await?;
}
Ok(plays)
}
/// }}}
// {{{ Tests
#[cfg(test)]
mod show_tests {
use super::*;
use crate::{commands::discord::mock::MockContext, with_test_ctx};
use std::path::PathBuf;
#[tokio::test]
async fn no_ids() -> Result<(), Error> {
with_test_ctx!("test/commands/score/show/no_ids", async |ctx| {
show_impl(ctx, &[]).await?;
Ok(())
})
}
#[tokio::test]
async fn nonexistent_id() -> Result<(), Error> {
with_test_ctx!("test/commands/score/show/nonexistent_id", async |ctx| {
show_impl(ctx, &[666]).await?;
Ok(())
})
}
#[tokio::test]
async fn agrees_with_magic() -> Result<(), Error> {
with_test_ctx!(
"test/commands/score/show/agrees_with_magic",
async |ctx: &mut MockContext| {
let created_plays = magic_impl(
ctx,
&[
PathBuf::from_str("test/screenshots/alter_ego.jpg")?,
PathBuf::from_str("test/screenshots/antithese_74_kerning.jpg")?,
PathBuf::from_str("test/screenshots/genocider_24_kerning.jpg")?,
],
)
.await?;
let ids = created_plays.iter().map(|p| p.id).collect::<Vec<_>>();
let plays = show_impl(ctx, &ids).await?;
assert_eq!(plays.len(), 3);
assert_eq!(created_plays, plays);
Ok(())
}
)
}
}
// }}}
// {{{ Discord wrapper
/// Show scores given their ides
#[poise::command(prefix_command, slash_command)]
pub async fn show(
mut ctx: Context<'_>,
#[description = "Ids of score to show"] ids: Vec<u32>,
) -> Result<(), Error> {
show_impl(&mut ctx, &ids).await?;
Ok(())
}
// }}}
// }}}
// {{{ Score delete
// {{{ Implementation
pub async fn delete_impl<C: MessageContext>(ctx: &mut C, ids: &[u32]) -> Result<(), Error> {
let user = get_user!(ctx);
if ids.len() == 0 {
ctx.reply("Empty ID list provided").await?;
return Ok(());
}
let mut count = 0;
for id in ids {
let res = ctx
.data()
.db
.get()?
.prepare_cached("DELETE FROM plays WHERE id=? AND user_id=?")?
.execute((id, user.id))?;
if res == 0 {
ctx.reply(&format!("No play with id {} found", id)).await?;
} else {
count += 1;
}
}
if count > 0 {
ctx.reply(&format!("Deleted {} play(s) successfully!", count))
.await?;
}
Ok(())
}
/// }}}
// {{{ Tests
#[cfg(test)]
mod delete_tests {
use super::*;
use crate::{
commands::discord::{mock::MockContext, play_song_title},
with_test_ctx,
};
use std::path::PathBuf;
#[tokio::test]
async fn no_ids() -> Result<(), Error> {
with_test_ctx!("test/commands/score/delete/no_ids", async |ctx| {
delete_impl(ctx, &[]).await?;
Ok(())
})
}
#[tokio::test]
async fn nonexistent_id() -> Result<(), Error> {
with_test_ctx!("test/commands/score/delete/nonexistent_id", async |ctx| {
delete_impl(ctx, &[666]).await?;
Ok(())
})
}
#[tokio::test]
async fn delete_twice() -> Result<(), Error> {
with_test_ctx!(
"test/commands/score/delete/delete_twice",
async |ctx: &mut MockContext| {
let plays =
magic_impl(ctx, &[PathBuf::from_str("test/screenshots/alter_ego.jpg")?])
.await?;
let id = plays[0].id;
delete_impl(ctx, &[id, id]).await?;
Ok(())
}
)
}
#[tokio::test]
async fn no_show_after_delete() -> Result<(), Error> {
with_test_ctx!(
"test/commands/score/delete/no_show_after_delete",
async |ctx: &mut MockContext| {
let plays =
magic_impl(ctx, &[PathBuf::from_str("test/screenshots/alter_ego.jpg")?])
.await?;
// Showcase proper usage
let ids = [plays[0].id];
delete_impl(ctx, &ids).await?;
// This will tell the user the play doesn't exist
let shown_plays = show_impl(ctx, &ids).await?;
assert_eq!(shown_plays.len(), 0);
Ok(())
}
)
}
#[tokio::test]
async fn delete_multiple() -> Result<(), Error> {
with_test_ctx!(
"test/commands/score/delete/delete_multiple",
async |ctx: &mut MockContext| {
let plays = magic_impl(
ctx,
&[
PathBuf::from_str("test/screenshots/antithese_74_kerning.jpg")?,
PathBuf::from_str("test/screenshots/alter_ego.jpg")?,
PathBuf::from_str("test/screenshots/genocider_24_kerning.jpg")?,
],
)
.await?;
delete_impl(ctx, &[plays[0].id, plays[2].id]).await?;
// Ensure the second play still exists
let shown_plays = show_impl(ctx, &[plays[1].id]).await?;
assert_eq!(play_song_title(ctx, &shown_plays[0])?, "ALTER EGO");
Ok(())
}
)
}
}
// }}}
// {{{ Discord wrapper
/// Delete scores, given their IDs.
#[poise::command(prefix_command, slash_command)]
pub async fn delete(
mut ctx: Context<'_>,
#[description = "Id of score to delete"] ids: Vec<u32>,
) -> Result<(), Error> {
delete_impl(&mut ctx, &ids).await?;
Ok(())
}
// }}}
// }}}