Skip to content

Commit

Permalink
fix(ci): Use llm-cov preview via nightly and improve test coverage (#289
Browse files Browse the repository at this point in the history
)

Fix test coverage in CI. Simplified the trait bounds on the query
pipeline for now to make it all work and fit together, and added more
tests to assert boxed versions of trait objects work in tests.
  • Loading branch information
timonv authored Sep 11, 2024
1 parent 1887755 commit 45d8a57
Show file tree
Hide file tree
Showing 9 changed files with 160 additions and 21 deletions.
10 changes: 4 additions & 6 deletions .github/workflows/coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,13 @@ jobs:
components: llvm-tools-preview
- name: Install Protoc
uses: arduino/setup-protoc@v3
- name: Cache Cargo dependencies
uses: Swatinem/rust-cache@v2
- 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 +nightly llvm-cov --workspace --all-features --lcov --output-path lcov.info
# - name: Coveralls
# uses: coverallsapp/github-action@v2
- name: Coveralls
uses: coverallsapp/github-action@v2
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ insta = { version = "1.39.0", features = ["yaml"] }

[workspace.lints.rust]
unsafe_code = "forbid"
unexpected_cfgs = { level = "warn", check-cfg = [
'cfg(coverage,coverage_nightly)',
] }

[workspace.lints.clippy]
cargo = { level = "warn", priority = -1 }
Expand Down
2 changes: 2 additions & 0 deletions swiftide-core/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![cfg_attr(coverage_nightly, feature(coverage_attribute))]

mod indexing_defaults;
mod indexing_stream;
pub mod indexing_traits;
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_attr(coverage_nightly, coverage(off))]

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();
}
}
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");
}
}

0 comments on commit 45d8a57

Please sign in to comment.