From 117a99741c19a73a0d32fc0633a1acfba09f8365 Mon Sep 17 00:00:00 2001 From: Timon Vonk Date: Tue, 10 Sep 2024 22:48:47 +0200 Subject: [PATCH 1/5] Improve test coverage --- Cargo.toml | 1 + swiftide-core/src/query.rs | 18 ++++++++-- swiftide-core/src/query_evaluation.rs | 49 +++++++++++++++++++++++++ swiftide-core/src/query_traits.rs | 8 +++++ swiftide-core/src/type_aliases.rs | 2 ++ swiftide-indexing/src/pipeline.rs | 38 ++++++++++++++++++++ swiftide-macros/src/lib.rs | 1 + swiftide-query/src/query/pipeline.rs | 51 ++++++++++++++++++++------- 8 files changed, 153 insertions(+), 15 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6e0ed49c..e434aae2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -71,6 +71,7 @@ insta = { version = "1.39.0", features = ["yaml"] } [workspace.lints.rust] unsafe_code = "forbid" +unexpected_cfgs = { level = "allow", check-cfg = ['cfg(tarpaulin_include)'] } [workspace.lints.clippy] cargo = { level = "warn", priority = -1 } diff --git a/swiftide-core/src/query.rs b/swiftide-core/src/query.rs index 62ed1af1..fa170c67 100644 --- a/swiftide-core/src/query.rs +++ b/swiftide-core/src/query.rs @@ -18,7 +18,7 @@ type Document = String; /// `states::Pending`: No documents have been retrieved /// `states::Retrieved`: Documents have been retrieved /// `states::Answered`: The query has been answered -#[derive(Clone, Default, Builder)] +#[derive(Clone, Default, Builder, PartialEq)] #[builder(setter(into))] pub struct Query { original: String, @@ -81,6 +81,10 @@ impl Query { } impl Query { + pub fn new() -> Self { + Self::default() + } + /// Transforms the current query pub fn transformed_query(&mut self, new_query: impl Into) { let new_query = new_query.into(); @@ -111,6 +115,10 @@ impl Query { } impl Query { + pub fn new() -> Self { + Self::default() + } + /// Transforms the current response pub fn transformed_response(&mut self, new_response: impl Into) { let new_response = new_response.into(); @@ -140,6 +148,10 @@ impl Query { } impl Query { + pub fn new() -> Self { + Self::default() + } + /// Returns the answer of the query pub fn answer(&self) -> &str { &self.state.answer @@ -155,13 +167,13 @@ pub mod states { /// The query is pending and has not been used pub struct Pending; - #[derive(Debug, Default, Clone, Builder)] + #[derive(Debug, Default, Clone, Builder, PartialEq)] #[builder(setter(into))] /// Documents have been retrieved pub struct Retrieved { pub(crate) documents: Vec, } - #[derive(Debug, Default, Clone, Builder)] + #[derive(Debug, Default, Clone, Builder, PartialEq)] #[builder(setter(into))] /// The query has been answered pub struct Answered { diff --git a/swiftide-core/src/query_evaluation.rs b/swiftide-core/src/query_evaluation.rs index 9060fc49..80cdd143 100644 --- a/swiftide-core/src/query_evaluation.rs +++ b/swiftide-core/src/query_evaluation.rs @@ -38,3 +38,52 @@ impl QueryEvaluation { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_from_retrieved() { + let query = Query::::new(); // Assuming Query has a new() method + let evaluation = QueryEvaluation::from(query.clone()); + + match evaluation { + QueryEvaluation::RetrieveDocuments(q) => assert_eq!(q, query), + QueryEvaluation::AnswerQuery(_) => panic!("Unexpected QueryEvaluation variant"), + } + } + + #[test] + fn test_from_answered() { + let query = Query::::new(); // Assuming Query has a new() method + let evaluation = QueryEvaluation::from(query.clone()); + + match evaluation { + QueryEvaluation::AnswerQuery(q) => assert_eq!(q, query), + QueryEvaluation::RetrieveDocuments(_) => panic!("Unexpected QueryEvaluation variant"), + } + } + + #[test] + fn test_retrieve_documents_query() { + let query = Query::::new(); // Assuming Query has a new() method + let evaluation = QueryEvaluation::RetrieveDocuments(query.clone()); + + match evaluation.retrieve_documents_query() { + Some(q) => assert_eq!(q, query), + None => panic!("Expected a query, got None"), + } + } + + #[test] + fn test_answer_query() { + let query = Query::::new(); // Assuming Query has a new() method + let evaluation = QueryEvaluation::AnswerQuery(query.clone()); + + match evaluation.answer_query() { + Some(q) => assert_eq!(q, query), + None => panic!("Expected a query, got None"), + } + } +} diff --git a/swiftide-core/src/query_traits.rs b/swiftide-core/src/query_traits.rs index f650c1b5..1242ef2a 100644 --- a/swiftide-core/src/query_traits.rs +++ b/swiftide-core/src/query_traits.rs @@ -9,7 +9,12 @@ use crate::{ querying::QueryEvaluation, }; +#[cfg(feature = "test-utils")] +#[doc(hidden)] +use mockall::{automock, predicate::str}; + /// Can transform queries before retrieval +#[cfg_attr(feature = "test-utils", automock)] #[async_trait] pub trait TransformQuery: Send + Sync { async fn transform_query( @@ -81,6 +86,7 @@ where } /// Can transform a response after retrieval +#[cfg_attr(feature = "test-utils", automock)] #[async_trait] pub trait TransformResponse: Send + Sync { async fn transform_response(&self, query: Query) @@ -105,6 +111,7 @@ impl TransformResponse for Box { } /// Can answer the original query +#[cfg_attr(feature = "test-utils", automock)] #[async_trait] pub trait Answer: Send + Sync { async fn answer(&self, query: Query) -> Result>; @@ -130,6 +137,7 @@ impl Answer for Box { /// Evaluates a query /// /// An evaluator needs to be able to respond to each step in the query pipeline +#[cfg_attr(feature = "test-utils", automock)] #[async_trait] pub trait EvaluateQuery: Send + Sync { async fn evaluate(&self, evaluation: QueryEvaluation) -> Result<()>; diff --git a/swiftide-core/src/type_aliases.rs b/swiftide-core/src/type_aliases.rs index 86c86d52..dc4190c4 100644 --- a/swiftide-core/src/type_aliases.rs +++ b/swiftide-core/src/type_aliases.rs @@ -1,3 +1,5 @@ +#![cfg(not(tarpaulin_include))] + use serde::{Deserialize, Serialize}; pub type Embedding = Vec; diff --git a/swiftide-indexing/src/pipeline.rs b/swiftide-indexing/src/pipeline.rs index fe1b8941..8e127305 100644 --- a/swiftide-indexing/src/pipeline.rs +++ b/swiftide-indexing/src/pipeline.rs @@ -825,4 +825,42 @@ mod tests { 2 ); } + + #[tokio::test] + async fn test_all_steps_should_work_as_dyn_box() { + let mut loader = MockLoader::new(); + loader + .expect_into_stream_boxed() + .returning(|| vec![Ok(Node::default())].into()); + + let mut transformer = MockTransformer::new(); + transformer.expect_transform_node().returning(Ok); + transformer.expect_concurrency().returning(|| None); + + let mut batch_transformer = MockBatchableTransformer::new(); + batch_transformer + .expect_batch_transform() + .returning(std::convert::Into::into); + batch_transformer.expect_concurrency().returning(|| None); + let mut chunker = MockChunkerTransformer::new(); + chunker + .expect_transform_node() + .returning(|node| vec![node].into()); + chunker.expect_concurrency().returning(|| None); + + let mut storage = MockPersist::new(); + storage.expect_setup().returning(|| Ok(())); + storage.expect_store().returning(Ok); + storage.expect_batch_size().returning(|| None); + + let pipeline = Pipeline::from_loader(Box::new(loader) as Box) + .then(Box::new(transformer) as Box) + .then_in_batch( + 1, + Box::new(batch_transformer) as Box, + ) + .then_chunk(Box::new(chunker) as Box) + .then_store_with(Box::new(storage) as Box); + pipeline.run().await.unwrap(); + } } diff --git a/swiftide-macros/src/lib.rs b/swiftide-macros/src/lib.rs index 65539411..4ee6f7ff 100644 --- a/swiftide-macros/src/lib.rs +++ b/swiftide-macros/src/lib.rs @@ -6,6 +6,7 @@ mod indexing_transformer; use indexing_transformer::indexing_transformer_impl; /// Generates boilerplate for an indexing transformer. +#[cfg(not(tarpaulin_include))] #[proc_macro_attribute] pub fn indexing_transformer(args: TokenStream, item: TokenStream) -> TokenStream { indexing_transformer_impl(args, item) diff --git a/swiftide-query/src/query/pipeline.rs b/swiftide-query/src/query/pipeline.rs index 11bf86e8..348203b7 100644 --- a/swiftide-query/src/query/pipeline.rs +++ b/swiftide-query/src/query/pipeline.rs @@ -80,22 +80,19 @@ where { /// Evaluate queries with an evaluator #[must_use] - pub fn evaluate_with>( - mut self, - evaluator: T, - ) -> Self { - self.evaluator = Some(Arc::new(Box::new(evaluator.to_owned()))); + pub fn evaluate_with(mut self, evaluator: T) -> Self { + self.evaluator = Some(Arc::new(Box::new(evaluator))); self } /// Transform a query into something else, see [`crate::query_transformers`] #[must_use] - pub fn then_transform_query>( + pub fn then_transform_query( self, transformer: T, ) -> Pipeline<'stream, S, states::Pending> { - let transformer = Arc::new(transformer.to_owned()); + let transformer = Arc::new(transformer); let Pipeline { stream, @@ -177,11 +174,11 @@ impl<'stream: 'static, S: SearchStrategy + 'stream> Pipeline<'stream, S, states: impl<'stream: 'static, S: SearchStrategy> Pipeline<'stream, S, states::Retrieved> { /// Transforms a retrieved query into something else #[must_use] - pub fn then_transform_response>( + pub fn then_transform_response( self, transformer: T, ) -> Pipeline<'stream, S, states::Retrieved> { - let transformer = Arc::new(transformer.to_owned()); + let transformer = Arc::new(transformer); let Pipeline { stream, query_sender, @@ -211,11 +208,11 @@ impl<'stream: 'static, S: SearchStrategy> Pipeline<'stream, S, states::Retrieved impl<'stream: 'static, S: SearchStrategy> Pipeline<'stream, S, states::Retrieved> { /// Generates an answer based on previous transformations #[must_use] - pub fn then_answer>( + pub fn then_answer( self, answerer: T, ) -> Pipeline<'stream, S, states::Answered> { - let answerer = Arc::new(answerer.to_owned()); + let answerer = Arc::new(answerer); let Pipeline { stream, query_sender, @@ -305,7 +302,9 @@ impl Pipeline<'_, S, states::Answered> { #[cfg(test)] mod test { - use swiftide_core::querying::search_strategies; + use swiftide_core::{ + querying::search_strategies, MockAnswer, MockTransformQuery, MockTransformResponse, + }; use super::*; @@ -324,4 +323,32 @@ mod test { let response = pipeline.query("What").await.unwrap(); assert_eq!(response.answer(), "Ok"); } + + #[tokio::test] + async fn test_all_steps_should_accept_dyn_box() { + let mut query_transformer = MockTransformQuery::new(); + query_transformer.expect_transform_query().returning(Ok); + + let mut response_transformer = MockTransformResponse::new(); + response_transformer + .expect_transform_response() + .returning(Ok); + let mut answer_transformer = MockAnswer::new(); + answer_transformer + .expect_answer() + .returning(|query| Ok(query.answered("OK"))); + + let pipeline = Pipeline::default() + .then_transform_query(Box::new(query_transformer) as Box) + .then_retrieve( + |_: &search_strategies::SimilaritySingleEmbedding, + query: Query| { + Ok(query.retrieved_documents(vec![])) + }, + ) + .then_transform_response(Box::new(response_transformer) as Box) + .then_answer(Box::new(answer_transformer) as Box); + let response = pipeline.query("What").await.unwrap(); + assert_eq!(response.answer(), "OK"); + } } From 0e71f7ebcd0daf649edd9322d763ded34d3605bd Mon Sep 17 00:00:00 2001 From: Timon Vonk Date: Tue, 10 Sep 2024 23:33:45 +0200 Subject: [PATCH 2/5] Switch back to llvm-cov --- .github/workflows/coverage.yml | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index b8f431ba..c8bb92cd 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -14,14 +14,17 @@ jobs: test: name: coverage runs-on: ubuntu-latest + env: + RUST_LOG: swiftide=debug + RUST_BACKTRACE: 1 steps: - name: Checkout repository uses: actions/checkout@v4 - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@nightly + - uses: dtolnay/rust-toolchain@stable with: - components: llvm-tools-preview + components: llvm-tools - name: Install Protoc uses: arduino/setup-protoc@v3 - name: Cache Cargo dependencies @@ -29,10 +32,10 @@ jobs: - name: Install cargo-llvm-cov uses: taiki-e/install-action@v2 with: - tool: cargo-tarpaulin + tool: cargo-llvm-cov - name: Generate code coverage run: | - cargo +nightly tarpaulin --workspace --all-features --tests --timeout 1200 --coveralls ${{ secrets.COVERALLS_REPO_TOKEN }} + cargo llvm-cov --lcov --output-path target/lcov.info --all-features --workspace - # - name: Coveralls - # uses: coverallsapp/github-action@v2 + - name: Coveralls + uses: coverallsapp/github-action@v2 From 6360a0d479e984c2be5f106f8b89612a781d3170 Mon Sep 17 00:00:00 2001 From: Timon Vonk Date: Wed, 11 Sep 2024 09:45:33 +0200 Subject: [PATCH 3/5] Revert "Switch back to llvm-cov" This reverts commit 0e71f7ebcd0daf649edd9322d763ded34d3605bd. --- .github/workflows/coverage.yml | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index c8bb92cd..b8f431ba 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -14,17 +14,14 @@ jobs: test: name: coverage runs-on: ubuntu-latest - env: - RUST_LOG: swiftide=debug - RUST_BACKTRACE: 1 steps: - name: Checkout repository uses: actions/checkout@v4 - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@stable + - uses: dtolnay/rust-toolchain@nightly with: - components: llvm-tools + components: llvm-tools-preview - name: Install Protoc uses: arduino/setup-protoc@v3 - name: Cache Cargo dependencies @@ -32,10 +29,10 @@ jobs: - name: Install cargo-llvm-cov uses: taiki-e/install-action@v2 with: - tool: cargo-llvm-cov + tool: cargo-tarpaulin - name: Generate code coverage run: | - cargo llvm-cov --lcov --output-path target/lcov.info --all-features --workspace + cargo +nightly tarpaulin --workspace --all-features --tests --timeout 1200 --coveralls ${{ secrets.COVERALLS_REPO_TOKEN }} - - name: Coveralls - uses: coverallsapp/github-action@v2 + # - name: Coveralls + # uses: coverallsapp/github-action@v2 From b40a6949c4d9e62fd7f12ab5a2300340703d2e47 Mon Sep 17 00:00:00 2001 From: Timon Vonk Date: Wed, 11 Sep 2024 09:49:38 +0200 Subject: [PATCH 4/5] Run on all targets --- .github/workflows/coverage.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index b8f431ba..9127d009 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -32,7 +32,7 @@ jobs: tool: cargo-tarpaulin - name: Generate code coverage run: | - cargo +nightly tarpaulin --workspace --all-features --tests --timeout 1200 --coveralls ${{ secrets.COVERALLS_REPO_TOKEN }} + cargo +nightly tarpaulin --workspace --all-features --all-targets --timeout 1200 --coveralls ${{ secrets.COVERALLS_REPO_TOKEN }} # - name: Coveralls # uses: coverallsapp/github-action@v2 From 3d735c6e441333feeeacca1f54fc81a16e819e9f Mon Sep 17 00:00:00 2001 From: Timon Vonk Date: Wed, 11 Sep 2024 11:25:27 +0200 Subject: [PATCH 5/5] Like this --- .github/workflows/coverage.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 9127d009..7bfbc007 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -32,7 +32,7 @@ jobs: tool: cargo-tarpaulin - name: Generate code coverage run: | - cargo +nightly tarpaulin --workspace --all-features --all-targets --timeout 1200 --coveralls ${{ secrets.COVERALLS_REPO_TOKEN }} + cargo +nightly tarpaulin --workspace --all-features --timeout 1200 --coveralls ${{ secrets.COVERALLS_REPO_TOKEN }} # - name: Coveralls # uses: coverallsapp/github-action@v2