Skip to content

Commit

Permalink
Merge pull request #112 from AnthonyMichaelTDM/82-feattesting-impleme…
Browse files Browse the repository at this point in the history
…nt-mock-daemon-clientserver-for-improved-testability

feat(daemon): test client that runs on channels instead of over tcp
  • Loading branch information
AnthonyMichaelTDM authored Sep 4, 2024
2 parents 11a377a + 004b1ba commit 890661b
Show file tree
Hide file tree
Showing 30 changed files with 607 additions and 534 deletions.
790 changes: 425 additions & 365 deletions Cargo.lock

Large diffs are not rendered by default.

18 changes: 9 additions & 9 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ opt-level = 0
[workspace.dependencies]
# shared dependencies
anyhow = { version = "1.0", default-features = false }
lofty = { version = "0.19.2" }
lofty = { version = "0.21.1" }
clap = { version = "4.5", default-features = false, features = [
"color",
"error-context",
Expand All @@ -88,7 +88,7 @@ notify = { version = "6.1", default-features = false, features = [
] }
once_cell = "1.19"
rand = { version = "0.8.5", features = ["small_rng"] }
rodio = { version = "0.18.1", features = ["symphonia-all"] }
rodio = { version = "0.19.0", features = ["symphonia-all"] }
rubato = { version = "0.15.0" }
serde = { version = "1.0", features = ["derive", "rc"] }
strum = { version = "0.26.3", features = ["derive"] }
Expand All @@ -98,7 +98,7 @@ surrealdb = { version = "1.5", features = [
], default-features = false }
tap = { version = "1.0" }
thiserror = { version = "1.0" }
tokio = { version = "1.37", features = ["macros", "rt-multi-thread"] }
tokio = { version = "1.40", features = ["macros", "rt-multi-thread"] }
walkdir = { version = "2.5" }
tarpc = { version = "0.34.0", features = [
"serde-transport",
Expand All @@ -107,11 +107,11 @@ tarpc = { version = "0.34.0", features = [
] }
tracing = { version = "0.1.40" }
tracing-subscriber = { version = "0.3.18", features = ["env-filter"] }
tracing-opentelemetry = "0.24.0"
opentelemetry = "0.23.0"
opentelemetry_sdk = { version = "0.23.0", features = ["rt-tokio"] }
opentelemetry-otlp = "0.16.0"
opentelemetry-semantic-conventions = { version = "0.15.0" }
tracing-opentelemetry = "0.25.0"
opentelemetry = "0.24.0"
opentelemetry_sdk = { version = "0.24.1", features = ["rt-tokio"] }
opentelemetry-otlp = "0.17.0"
opentelemetry-semantic-conventions = { version = "0.16.0" }
tracing-flame = "0.2.0"

# MECOMP packages
Expand All @@ -124,7 +124,7 @@ one-or-many = { path = "one-or-many" }

# shared dev dependencies
pretty_assertions = "1.4"
rstest = "0.21.0"
rstest = "0.22.0"
rstest_reuse = { version = "0.7.0" }
tempfile = { version = "3.10" }
criterion = { version = "0.5.1", features = ["html_reports"] }
Expand Down
2 changes: 1 addition & 1 deletion Mecomp.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ library_paths = ["~/Music"]
## For example, "Foo, Bar, Baz" would be split into ["Foo", "Bar", "Baz"]. if the separator is ", ".
## If the separator is not found, the entire string is considered as a single artist.
## If unset, will not split artists.
artist_separator = ", "
artist_separator = "; "
## Separators for genres in song metadata.
## For example, "Foo, Bar, Baz" would be split into ["Foo", "Bar", "Baz"]. if the separator is ", ".
## If the separator is not found, the entire string is considered as a single genre.
Expand Down
2 changes: 1 addition & 1 deletion analysis/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ rand.workspace = true
rayon = "1.10"
rodio = { workspace = true }
rubato = { workspace = true }
rustfft = { version = "6.1" }
rustfft = { version = "6.2" }
serde = { workspace = true }
statrs = "0.17.1"
strum.workspace = true
Expand Down
5 changes: 3 additions & 2 deletions analysis/src/chroma.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,9 @@ pub fn chroma_filter(
uninit.set_len(wts.len());
}
let mut b = Array::from(uninit)
.into_shape(wts.dim())
.map_err(|e| AnalysisError::AnalysisError(format!("in chroma: {e}")))?;
.to_shape(wts.dim())
.map_err(|e| AnalysisError::AnalysisError(format!("in chroma: {e}")))?
.to_owned();
b.slice_mut(s![-3.., ..]).assign(&wts.slice(s![..3, ..]));
b.slice_mut(s![..-3, ..]).assign(&wts.slice(s![3.., ..]));

Expand Down
5 changes: 3 additions & 2 deletions analysis/src/clustering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,9 @@ fn convert_to_array<T: Sample>(data: &[T]) -> Array2<f64> {
let shape = (data.len(), NUMBER_FEATURES);
//let mut array = Array2::zeros(shape);
let data = Array1::from_iter(data.iter().flat_map(|v| *v.inner()))
.into_shape(shape)
.expect("Failed to reshape!");
.to_shape(shape)
.expect("Failed to reshape!")
.to_owned();
data
}

Expand Down
5 changes: 4 additions & 1 deletion core/src/audio/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,10 @@ impl AudioKernel {
let duration_info = self.duration_info.clone();
let paused = self.paused.clone();

let _duration_water = std::thread::Builder::new().name(String::from("Duration Watcher")).spawn(move || {
// NOTE: as of rodio v0.19.0, we have access to the `get_pos` command, which allows us to get the current position of the audio stream
// it may seem like this means we don't need to have a duration watcher, but the key point is that we need to know when to skip to the next song
// the duration watcher both tracks the duration of the song, and skips to the next song when the song is over
let _duration_watcher = std::thread::Builder::new().name(String::from("Duration Watcher")).spawn(move || {
let sleep_time = std::time::Duration::from_millis(DURATION_WATCHER_TICK_MS);
let duration_threshold =
std::time::Duration::from_millis(DURATION_WATCHER_NEXT_SONG_THRESHOLD_MS);
Expand Down
7 changes: 5 additions & 2 deletions core/src/logger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use std::time::Instant;
use log::info;
use once_cell::sync::Lazy;
#[cfg(feature = "otel_tracing")]
use opentelemetry::trace::TracerProvider as _;
#[cfg(feature = "otel_tracing")]
use opentelemetry::KeyValue;
#[cfg(feature = "otel_tracing")]
use opentelemetry_otlp::WithExportConfig as _;
Expand Down Expand Up @@ -159,7 +161,7 @@ pub fn init_tracing() -> impl tracing::Subscriber {
.with_endpoint("http://localhost:4317"),
)
.with_trace_config(
opentelemetry_sdk::trace::config()
opentelemetry_sdk::trace::Config::default()
.with_resource(Resource::new(vec![KeyValue::new(
opentelemetry_semantic_conventions::resource::SERVICE_NAME,
"mecomp-daemon",
Expand All @@ -168,7 +170,8 @@ pub fn init_tracing() -> impl tracing::Subscriber {
.with_sampler(opentelemetry_sdk::trace::Sampler::AlwaysOn),
)
.install_batch(opentelemetry_sdk::runtime::Tokio)
.expect("Failed to create tracing layer");
.expect("Failed to create tracing layer")
.tracer("mecomp-daemon");

#[cfg(feature = "otel_tracing")]
let subscriber = subscriber.with(
Expand Down
5 changes: 1 addition & 4 deletions daemon/src/controller.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//----------------------------------------------------------------------------------------- std lib
use std::{net::SocketAddr, ops::Range, sync::Arc, time::Duration};
use std::{ops::Range, sync::Arc, time::Duration};
//--------------------------------------------------------------------------------- other libraries
use ::tarpc::context::Context;
use log::{debug, error, info, warn};
Expand Down Expand Up @@ -48,7 +48,6 @@ mod locks {

#[derive(Clone, Debug)]
pub struct MusicPlayerServer {
pub addr: SocketAddr,
db: Arc<Surreal<Db>>,
settings: Arc<Settings>,
audio_kernel: Arc<AudioKernelSender>,
Expand All @@ -57,13 +56,11 @@ pub struct MusicPlayerServer {
impl MusicPlayerServer {
#[must_use]
pub fn new(
addr: SocketAddr,
db: Arc<Surreal<Db>>,
settings: Arc<Settings>,
audio_kernel: Arc<AudioKernelSender>,
) -> Self {
Self {
addr,
db,
settings,
audio_kernel,
Expand Down
57 changes: 51 additions & 6 deletions daemon/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::{
//--------------------------------------------------------------------------------- other libraries
use futures::{future, prelude::*};
use log::info;
use surrealdb::{engine::local::Db, Surreal};
use tarpc::{
self,
server::{incoming::Incoming as _, BaseChannel, Channel as _},
Expand All @@ -16,6 +17,7 @@ use mecomp_core::{
audio::AudioKernelSender,
logger::{init_logger, init_tracing},
rpc::MusicPlayer as _,
rpc::MusicPlayerClient,
};
use mecomp_storage::db::{init_database, set_database_path};

Expand Down Expand Up @@ -90,12 +92,7 @@ pub async fn start_daemon(settings: Settings, db_dir: std::path::PathBuf) -> any
// serve is generated by the service attribute.
// It takes as input any type implementing the generated MusicPlayer trait.
.map(|channel| {
let server = MusicPlayerServer::new(
channel.transport().peer_addr().unwrap(),
db.clone(),
settings.clone(),
audio_kernel.clone(),
);
let server = MusicPlayerServer::new(db.clone(), settings.clone(), audio_kernel.clone());
channel.execute(server.serve()).for_each(spawn)
})
// Max 10 channels.
Expand All @@ -111,3 +108,51 @@ pub async fn start_daemon(settings: Settings, db_dir: std::path::PathBuf) -> any

Ok(())
}

/// Initialize a test client, sends and receives messages over a channel / pipe.
/// This is useful for testing the server without needing to start it.
#[must_use]
pub fn init_test_client_server(
db: Arc<Surreal<Db>>,
settings: Arc<Settings>,
audio_kernel: Arc<AudioKernelSender>,
) -> MusicPlayerClient {
let (client_transport, server_transport) = tarpc::transport::channel::unbounded();

let server = MusicPlayerServer::new(db, settings, audio_kernel);
tokio::spawn(
tarpc::server::BaseChannel::with_defaults(server_transport)
.execute(server.serve())
// Handle all requests concurrently.
.for_each(|response| async move {
tokio::spawn(response);
}),
);

// MusicPlayerClient is generated by the #[tarpc::service] attribute. It has a constructor `new`
// that takes a config and any Transport as input.
MusicPlayerClient::new(tarpc::client::Config::default(), client_transport).spawn()
}

#[cfg(test)]
mod test_client_tests {
use super::*;
use mecomp_storage::test_utils::init_test_database;

#[tokio::test]
async fn test_init_test_client_server() {
let db = Arc::new(init_test_database().await.unwrap());
let settings = Arc::new(Settings::default());
let audio_kernel = AudioKernelSender::start();

let client = init_test_client_server(db, settings, audio_kernel);

let ctx = tarpc::context::current();
let response = client.ping(ctx).await.unwrap();

assert_eq!(response, "pong");

// ensure that the client is shutdown properly
drop(client);
}
}
22 changes: 17 additions & 5 deletions mecomp-workspace-hack/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ ignored = [
"anyhow",
"approx",
"arrayvec",
"base64",
"byteorder",
"clap",
"clap_builder",
"criterion",
Expand All @@ -33,7 +35,6 @@ ignored = [
"getrandom",
"hashbrown",
"indexmap",
"itertools-5ef9efb8ec2df382",
"itertools-93f6ce9d446188ac",
"itertools-a6292c17cd707f01",
"lalrpop-util",
Expand All @@ -54,10 +55,13 @@ ignored = [
"semver",
"serde",
"serde_json",
"smallvec",
"syn",
"time",
"toml_datetime",
"toml_edit",
"tokio",
"tokio-stream",
"tokio-util",
"tracing",
"tracing-core",
Expand All @@ -77,6 +81,8 @@ ahash = { version = "0.8" }
anyhow = { version = "1" }
approx = { version = "0.5" }
arrayvec = { version = "0.7" }
base64 = { version = "0.22" }
byteorder = { version = "1" }
clap = { version = "4", default-features = false, features = ["color", "derive", "error-context", "help", "std", "suggestions", "usage"] }
clap_builder = { version = "4", default-features = false, features = ["color", "help", "std", "suggestions", "usage"] }
criterion = { version = "0.5", features = ["async_tokio", "html_reports"] }
Expand All @@ -88,11 +94,10 @@ futures-core = { version = "0.3" }
futures-executor = { version = "0.3" }
futures-io = { version = "0.3" }
futures-sink = { version = "0.3" }
futures-util = { version = "0.3", features = ["channel", "io", "sink"] }
futures-util = { version = "0.3", default-features = false, features = ["async-await-macro", "channel", "io", "sink"] }
getrandom = { version = "0.2", default-features = false, features = ["js", "rdrand", "std"] }
hashbrown = { version = "0.14", features = ["raw", "serde"] }
indexmap = { version = "2", features = ["serde"] }
itertools-5ef9efb8ec2df382 = { package = "itertools", version = "0.12", default-features = false, features = ["use_alloc"] }
itertools-93f6ce9d446188ac = { package = "itertools", version = "0.10" }
itertools-a6292c17cd707f01 = { package = "itertools", version = "0.11" }
lalrpop-util = { version = "0.20", features = ["lexer"] }
Expand All @@ -113,11 +118,14 @@ regex-syntax = { version = "0.8" }
semver = { version = "1", features = ["serde"] }
serde = { version = "1", features = ["alloc", "derive", "rc"] }
serde_json = { version = "1", features = ["alloc", "preserve_order"] }
smallvec = { version = "1", default-features = false, features = ["const_new"] }
syn = { version = "2", features = ["extra-traits", "full", "visit", "visit-mut"] }
time = { version = "0.3", features = ["formatting", "local-offset", "macros", "parsing"] }
tokio = { version = "1", features = ["io-std", "io-util", "macros", "net", "rt-multi-thread", "signal", "sync", "time"] }
tokio-stream = { version = "0.1", features = ["net"] }
tokio-util = { version = "0.7", features = ["codec", "io", "time"] }
toml_datetime = { version = "0.6", default-features = false, features = ["serde"] }
toml_edit = { version = "0.22", features = ["serde"] }
tracing = { version = "0.1", features = ["log"] }
tracing-core = { version = "0.1" }
ulid = { version = "1", features = ["serde"] }
Expand All @@ -128,6 +136,8 @@ ahash = { version = "0.8" }
anyhow = { version = "1" }
approx = { version = "0.5" }
arrayvec = { version = "0.7" }
base64 = { version = "0.22" }
byteorder = { version = "1" }
cc = { version = "1", default-features = false, features = ["parallel"] }
clap = { version = "4", default-features = false, features = ["color", "derive", "error-context", "help", "std", "suggestions", "usage"] }
clap_builder = { version = "4", default-features = false, features = ["color", "help", "std", "suggestions", "usage"] }
Expand All @@ -140,11 +150,10 @@ futures-core = { version = "0.3" }
futures-executor = { version = "0.3" }
futures-io = { version = "0.3" }
futures-sink = { version = "0.3" }
futures-util = { version = "0.3", features = ["channel", "io", "sink"] }
futures-util = { version = "0.3", default-features = false, features = ["async-await-macro", "channel", "io", "sink"] }
getrandom = { version = "0.2", default-features = false, features = ["js", "rdrand", "std"] }
hashbrown = { version = "0.14", features = ["raw", "serde"] }
indexmap = { version = "2", features = ["serde"] }
itertools-5ef9efb8ec2df382 = { package = "itertools", version = "0.12", default-features = false, features = ["use_alloc"] }
itertools-93f6ce9d446188ac = { package = "itertools", version = "0.10" }
itertools-a6292c17cd707f01 = { package = "itertools", version = "0.11" }
lalrpop-util = { version = "0.20", features = ["lexer"] }
Expand All @@ -165,11 +174,14 @@ regex-syntax = { version = "0.8" }
semver = { version = "1", features = ["serde"] }
serde = { version = "1", features = ["alloc", "derive", "rc"] }
serde_json = { version = "1", features = ["alloc", "preserve_order"] }
smallvec = { version = "1", default-features = false, features = ["const_new"] }
syn = { version = "2", features = ["extra-traits", "full", "visit", "visit-mut"] }
time = { version = "0.3", features = ["formatting", "local-offset", "macros", "parsing"] }
tokio = { version = "1", features = ["io-std", "io-util", "macros", "net", "rt-multi-thread", "signal", "sync", "time"] }
tokio-stream = { version = "0.1", features = ["net"] }
tokio-util = { version = "0.7", features = ["codec", "io", "time"] }
toml_datetime = { version = "0.6", default-features = false, features = ["serde"] }
toml_edit = { version = "0.22", features = ["serde"] }
tracing = { version = "0.1", features = ["log"] }
tracing-core = { version = "0.1" }
ulid = { version = "1", features = ["serde"] }
Expand Down
4 changes: 2 additions & 2 deletions tui/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ bench = false
[dependencies]
anyhow.workspace = true
clap.workspace = true
crossterm = { version = "0.27.0", features = ["event-stream"] }
ratatui = { version = "0.27.0", features = ["all-widgets"] }
crossterm = { version = "0.28.1", features = ["event-stream"] }
ratatui = { version = "0.28.1", features = ["all-widgets"] }
# log.workspace = true
tarpc.workspace = true
tokio = { workspace = true, features = ["signal"] }
Expand Down
9 changes: 6 additions & 3 deletions tui/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use mecomp_storage::db::schemas::{
album::Album, artist::Artist, collection::Collection, playlist::Playlist, song::Song, Id, Thing,
};
use one_or_many::OneOrMany;
use ratatui::{backend::TestBackend, Terminal};
use ratatui::{backend::TestBackend, layout::Rect, Terminal};

use crate::ui::{
app::ActiveComponent,
Expand All @@ -19,9 +19,12 @@ use crate::ui::{
/// # Panics
///
/// Panics if the terminal cannot be created.
pub fn setup_test_terminal(width: u16, height: u16) -> Terminal<TestBackend> {
pub fn setup_test_terminal(width: u16, height: u16) -> (Terminal<TestBackend>, Rect) {
let backend = TestBackend::new(width, height);
Terminal::new(backend).unwrap()
(
Terminal::new(backend).unwrap(),
Rect::new(0, 0, width, height),
)
}

/// check if the area and content (raw text) of two buffers are the same
Expand Down
Loading

0 comments on commit 890661b

Please sign in to comment.