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

fix(ci): Use llm-cov preview via nightly and improve test coverage #289

Merged
merged 11 commits into from
Sep 11, 2024
Merged
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
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");
}
}
Loading