Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Improve test coverage #287

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 --timeout 1200 --coveralls ${{ secrets.COVERALLS_REPO_TOKEN }}

# - name: Coveralls
# uses: coverallsapp/github-action@v2
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
18 changes: 15 additions & 3 deletions swiftide-core/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<State> {
original: String,
Expand Down Expand Up @@ -81,6 +81,10 @@ impl<T: Clone> Query<T> {
}

impl Query<states::Pending> {
pub fn new() -> Self {
Self::default()
}

/// Transforms the current query
pub fn transformed_query(&mut self, new_query: impl Into<String>) {
let new_query = new_query.into();
Expand Down Expand Up @@ -111,6 +115,10 @@ impl Query<states::Pending> {
}

impl Query<states::Retrieved> {
pub fn new() -> Self {
Self::default()
}

/// Transforms the current response
pub fn transformed_response(&mut self, new_response: impl Into<String>) {
let new_response = new_response.into();
Expand Down Expand Up @@ -140,6 +148,10 @@ impl Query<states::Retrieved> {
}

impl Query<states::Answered> {
pub fn new() -> Self {
Self::default()
}

/// Returns the answer of the query
pub fn answer(&self) -> &str {
&self.state.answer
Expand All @@ -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<Document>,
}
#[derive(Debug, Default, Clone, Builder)]
#[derive(Debug, Default, Clone, Builder, PartialEq)]
#[builder(setter(into))]
/// The query has been answered
pub struct Answered {
Expand Down
49 changes: 49 additions & 0 deletions swiftide-core/src/query_evaluation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,52 @@ impl QueryEvaluation {
}
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_from_retrieved() {
let query = Query::<states::Retrieved>::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::<states::Answered>::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::<states::Retrieved>::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::<states::Answered>::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"),
}
}
}
8 changes: 8 additions & 0 deletions swiftide-core/src/query_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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<Retrieved>)
Expand All @@ -105,6 +111,7 @@ impl TransformResponse for Box<dyn TransformResponse> {
}

/// Can answer the original query
#[cfg_attr(feature = "test-utils", automock)]
#[async_trait]
pub trait Answer: Send + Sync {
async fn answer(&self, query: Query<states::Retrieved>) -> Result<Query<states::Answered>>;
Expand All @@ -130,6 +137,7 @@ impl Answer for Box<dyn Answer> {
/// 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<()>;
Expand Down
2 changes: 2 additions & 0 deletions swiftide-core/src/type_aliases.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![cfg(not(tarpaulin_include))]

use serde::{Deserialize, Serialize};

pub type Embedding = Vec<f32>;
Expand Down
38 changes: 38 additions & 0 deletions swiftide-indexing/src/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<dyn Loader>)
.then(Box::new(transformer) as Box<dyn Transformer>)
.then_in_batch(
1,
Box::new(batch_transformer) as Box<dyn BatchableTransformer>,
)
.then_chunk(Box::new(chunker) as Box<dyn ChunkerTransformer>)
.then_store_with(Box::new(storage) as Box<dyn Persist>);
pipeline.run().await.unwrap();
}
}
1 change: 1 addition & 0 deletions swiftide-macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
51 changes: 39 additions & 12 deletions swiftide-query/src/query/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,22 +80,19 @@ where
{
/// Evaluate queries with an evaluator
#[must_use]
pub fn evaluate_with<T: ToOwned<Owned = impl EvaluateQuery + 'stream>>(
mut self,
evaluator: T,
) -> Self {
self.evaluator = Some(Arc::new(Box::new(evaluator.to_owned())));
pub fn evaluate_with<T: EvaluateQuery + 'stream>(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<T: ToOwned<Owned = impl TransformQuery + 'stream>>(
pub fn then_transform_query<T: TransformQuery + 'stream>(
self,
transformer: T,
) -> Pipeline<'stream, S, states::Pending> {
let transformer = Arc::new(transformer.to_owned());
let transformer = Arc::new(transformer);

let Pipeline {
stream,
Expand Down Expand Up @@ -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<T: ToOwned<Owned = impl TransformResponse + 'stream>>(
pub fn then_transform_response<T: TransformResponse + 'stream>(
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,
Expand Down Expand Up @@ -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<T: ToOwned<Owned = impl Answer + 'stream>>(
pub fn then_answer<T: Answer + 'stream>(
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,
Expand Down Expand Up @@ -305,7 +302,9 @@ impl<S: SearchStrategy> 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::*;

Expand All @@ -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<dyn TransformQuery>)
.then_retrieve(
|_: &search_strategies::SimilaritySingleEmbedding,
query: Query<states::Pending>| {
Ok(query.retrieved_documents(vec![]))
},
)
.then_transform_response(Box::new(response_transformer) as Box<dyn TransformResponse>)
.then_answer(Box::new(answer_transformer) as Box<dyn Answer>);
let response = pipeline.query("What").await.unwrap();
assert_eq!(response.answer(), "OK");
}
}