Skip to content

Commit

Permalink
FIX BUILD: Fix code coverage and some improvements (langchain4j#2010)
Browse files Browse the repository at this point in the history
## Issue
Build was failing due to low test coverage after [this
change](https://github.com/langchain4j/langchain4j/pull/1987/files#diff-9a5519a26a4b6d2fd412c877de4459c2a119e793fece9385f558a93a7e0aee5aR273-R275).
The hotfix is to enable tracing for logger with mockStatic for in the
affected DefaultRetrievalAugmentorTest.

Other changes:

* **Refine integration test run conditions to exclude experimental
builds and require the presence of the `OPENAI_API_KEY` secret.**
* [Upgrade mockito
instrumentation](https://javadoc.io/doc/org.mockito/mockito-core/latest/org/mockito/Mockito.html#0.3)
(javaagent) setup to make it compatible with new JDKs
* Updated `pom.xml` to upgrade `jacoco-maven-plugin` version. 
* Enhanced `README.md` with additional badges for nightly build and
Codacy dashboard.
(`[README.mdL3-R6](diffhunk://#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5L3-R6)`)
* Introduced new tests for comparison filters
* Added tests for logical filters
* Added configuration for external dependencies in
`.idea/externalDependencies.xml` to include SonarLint and SpotBugs
plugins.

## General checklist
- [x] There are no breaking changes
- [x] I have added unit and integration tests for my change
- [ ] I have manually run all the unit and integration tests in the
module I have added/changed, and they are all green
- [ ] I have manually run all the unit and integration tests in the
[core](https://github.com/langchain4j/langchain4j/tree/main/langchain4j-core)
and
[main](https://github.com/langchain4j/langchain4j/tree/main/langchain4j)
modules, and they are all green
- [ ] I have added/updated the
[documentation](https://github.com/langchain4j/langchain4j/tree/main/docs/docs)
- [ ] I have added an example in the [examples
repo](https://github.com/langchain4j/langchain4j-examples) (only for
"big" features)
- [ ] I have added/updated [Spring Boot
starter(s)](https://github.com/langchain4j/langchain4j-spring) (if
applicable)
  • Loading branch information
kpavlov authored Oct 31, 2024
1 parent aa0e488 commit 9178367
Show file tree
Hide file tree
Showing 20 changed files with 650 additions and 80 deletions.
11 changes: 6 additions & 5 deletions .github/workflows/main.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ jobs:
- java_version: 23
mvn_opts: ''
experimental: true
env:
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
steps:
- uses: actions/checkout@v4
with:
Expand All @@ -69,11 +71,10 @@ jobs:
${{ matrix.mvn_opts }}
- name: Integration test with JDK ${{ matrix.java_version }}
## allow builds to run only:
## - on the main branch when it is not a pull request,
## - for pull requests from the same repository
## - when triggered manually
if: ${{ !matrix.experimental }} && (github.event_name == 'pull_request' && github.head_repo.full_name == github.repository) || (github.event_name == 'push' && github.ref == 'refs/heads/main') || github.event_name == 'workflow_dispatch'
## The step or job will only run if the `experimental` variable
## in the matrix is false (not set to true)
## and the OPENAI_API_KEY secret is available and not empty.
if: ${{ !matrix.experimental && env.OPENAI_API_KEY != '' }}
run: |
mvn -B -U verify \
-Dgib.disable=false -Dgib.referenceBranch=__branch_before \
Expand Down
7 changes: 7 additions & 0 deletions .idea/externalDependencies.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# LangChain for Java: Supercharge your Java application with the power of LLMs

[![Build Status](https://img.shields.io/github/actions/workflow/status/langchain4j/langchain4j/main.yaml?branch=main&style=for-the-badge&label=GITHUB%20ACTIONS&logo=github)](https://github.com/langchain4j/langchain4j/actions/workflows/main.yaml)
[![Build Status](https://img.shields.io/github/actions/workflow/status/langchain4j/langchain4j/main.yaml?branch=main&style=for-the-badge&label=CI%20BUILD&logo=github)](https://github.com/langchain4j/langchain4j/actions/workflows/main.yaml)
[![Nightly Build](https://img.shields.io/github/actions/workflow/status/langchain4j/langchain4j/nightly.yaml?branch=main&style=for-the-badge&label=NIGHTLY%20BUILD&logo=github)](https://github.com/langchain4j/langchain4j/actions/workflows/nightly.yaml)
[![CODACY](https://img.shields.io/badge/Codacy-Dashboard-blue?style=for-the-badge&logo=codacy)](https://app.codacy.com/gh/langchain4j/langchain4j/dashboard)

[![Discord](https://dcbadge.vercel.app/api/server/JzTFvyjG6R?style=for-the-badge)](https://discord.gg/JzTFvyjG6R)
[![X](https://img.shields.io/badge/@langchain4j-follow-blue?logo=x&style=for-the-badge)](https://x.com/langchain4j)
[![Maven Version](https://img.shields.io/maven-central/v/dev.langchain4j/langchain4j?logo=apachemaven&style=for-the-badge)](https://search.maven.org/#search|gav|1|g:"dev.langchain4j"%20AND%20a:"langchain4j")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ public class GraalVmJavaScriptExecutionEngine implements CodeExecutionEngine {
public String execute(String code) {
OutputStream outputStream = new ByteArrayOutputStream();
try (Context context = Context.newBuilder("js")
.sandbox(CONSTRAINED)
.allowHostAccess(UNTRUSTED)
.out(outputStream)
.err(outputStream)
.build()) {
.sandbox(CONSTRAINED)
.allowHostAccess(UNTRUSTED)
.out(outputStream)
.err(outputStream)
.build()) {
Object result = context.eval("js", code).as(Object.class);
return String.valueOf(result);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ public class GraalVmPythonExecutionEngine implements CodeExecutionEngine {
public String execute(String code) {
OutputStream outputStream = new ByteArrayOutputStream();
try (Context context = Context.newBuilder("python")
.sandbox(TRUSTED)
.allowHostAccess(UNTRUSTED)
.out(outputStream)
.err(outputStream)
.build()) {
.sandbox(TRUSTED)
.allowHostAccess(UNTRUSTED)
.out(outputStream)
.err(outputStream)
.build()) {
Object result = context.eval("python", code).as(Object.class);
return String.valueOf(result);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ class Judge0JavaScriptEngine implements CodeExecutionEngine {
this.apiKey = apiKey;
this.languageId = languageId;
this.client = new OkHttpClient.Builder()
.connectTimeout(timeout)
.readTimeout(timeout)
.writeTimeout(timeout)
.callTimeout(timeout)
.build();
.connectTimeout(timeout)
.readTimeout(timeout)
.writeTimeout(timeout)
.callTimeout(timeout)
.build();
}

@Override
Expand All @@ -42,10 +42,10 @@ public String execute(String code) {
RequestBody requestBody = RequestBody.create(Json.toJson(submission), MEDIA_TYPE);

Request request = new Request.Builder()
.url("https://judge0-ce.p.rapidapi.com/submissions?base64_encoded=true&wait=true&fields=*")
.addHeader("X-RapidAPI-Key", apiKey)
.post(requestBody)
.build();
.url("https://judge0-ce.p.rapidapi.com/submissions?base64_encoded=true&wait=true&fields=*")
.addHeader("X-RapidAPI-Key", apiKey)
.post(requestBody)
.build();

try {
Response response = client.newCall(request).execute();
Expand Down
21 changes: 19 additions & 2 deletions langchain4j-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
<artifactId>slf4j-api</artifactId>
</dependency>

<!-- Test dependencies -->
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
Expand Down Expand Up @@ -85,7 +86,6 @@

<build>
<plugins>

<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-jar-plugin</artifactId>
Expand All @@ -101,7 +101,7 @@
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
<version>0.8.11</version>
<version>0.8.12</version>
<executions>
<execution>
<id>prepare-agent</id>
Expand Down Expand Up @@ -209,4 +209,21 @@
</license>
</licenses>

<reporting>
<plugins>
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
<reportSets>
<reportSet>
<reports>
<!-- select non-aggregate reports -->
<report>report</report>
</reports>
</reportSet>
</reportSets>
</plugin>
</plugins>
</reporting>

</project>
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,14 @@
import dev.langchain4j.rag.query.router.QueryRouter;
import dev.langchain4j.rag.query.transformer.DefaultQueryTransformer;
import dev.langchain4j.rag.query.transformer.QueryTransformer;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
import org.mockito.MockedStatic;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.Collection;
import java.util.HashMap;
Expand All @@ -32,14 +37,31 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;

class DefaultRetrievalAugmentorTest {

private static MockedStatic<LoggerFactory> loggerFactoryMock;

@BeforeAll
static void mockLogger() {
loggerFactoryMock = mockStatic(LoggerFactory.class);
Logger logger = mock(Logger.class);
when(LoggerFactory.getLogger(DefaultRetrievalAugmentor.class)).thenReturn(logger);
when(logger.isTraceEnabled()).thenReturn(true);
}

@AfterAll
static void releaseLogger() {
loggerFactoryMock.close();
}

@ParameterizedTest
@MethodSource("executors")
void should_augment_user_message__multiple_queries_multiple_retrievers(Executor executor) {
Expand All @@ -64,12 +86,12 @@ void should_augment_user_message__multiple_queries_multiple_retrievers(Executor
ContentInjector contentInjector = spy(new TestContentInjector());

RetrievalAugmentor retrievalAugmentor = DefaultRetrievalAugmentor.builder()
.queryTransformer(queryTransformer)
.queryRouter(queryRouter)
.contentAggregator(contentAggregator)
.contentInjector(contentInjector)
.executor(executor)
.build();
.queryTransformer(queryTransformer)
.queryRouter(queryRouter)
.contentAggregator(contentAggregator)
.contentInjector(contentInjector)
.executor(executor)
.build();

UserMessage userMessage = UserMessage.from("query");

Expand All @@ -80,7 +102,7 @@ void should_augment_user_message__multiple_queries_multiple_retrievers(Executor

// then
assertThat(augmented.singleText()).isEqualTo(
"""
"""
query
content 1
content 2
Expand Down Expand Up @@ -109,25 +131,25 @@ void should_augment_user_message__multiple_queries_multiple_retrievers(Executor

Map<Query, Collection<List<Content>>> queryToContents = new HashMap<>();
queryToContents.put(query1, asList(
asList(content1, content2),
asList(content3, content4)
asList(content1, content2),
asList(content3, content4)

));
queryToContents.put(query2, asList(
asList(content1, content2),
asList(content3, content4)
asList(content1, content2),
asList(content3, content4)

));
verify(contentAggregator).aggregate(queryToContents);
verifyNoMoreInteractions(contentAggregator);

verify(contentInjector).inject(asList(
content1, content2, content3, content4,
content1, content2, content3, content4
content1, content2, content3, content4,
content1, content2, content3, content4
), userMessage);
verify(contentInjector).inject(asList(
content1, content2, content3, content4,
content1, content2, content3, content4
content1, content2, content3, content4,
content1, content2, content3, content4
), (ChatMessage) userMessage);
verifyNoMoreInteractions(contentInjector);
}
Expand Down Expand Up @@ -155,12 +177,12 @@ void should_augment_user_message__single_query_multiple_retrievers() {
Executor executor = spy(new TestExecutor());

RetrievalAugmentor retrievalAugmentor = DefaultRetrievalAugmentor.builder()
.queryTransformer(queryTransformer)
.queryRouter(queryRouter)
.contentAggregator(contentAggregator)
.contentInjector(contentInjector)
.executor(executor)
.build();
.queryTransformer(queryTransformer)
.queryRouter(queryRouter)
.contentAggregator(contentAggregator)
.contentInjector(contentInjector)
.executor(executor)
.build();

UserMessage userMessage = UserMessage.from("query");

Expand All @@ -171,7 +193,7 @@ void should_augment_user_message__single_query_multiple_retrievers() {

// then
assertThat(augmented.singleText()).isEqualTo(
"""
"""
query
content 1
content 2
Expand All @@ -194,8 +216,8 @@ void should_augment_user_message__single_query_multiple_retrievers() {

Map<Query, Collection<List<Content>>> queryToContents = new HashMap<>();
queryToContents.put(query, asList(
asList(content1, content2),
asList(content3, content4)
asList(content1, content2),
asList(content3, content4)

));
verify(contentAggregator).aggregate(queryToContents);
Expand Down Expand Up @@ -236,12 +258,12 @@ void should_augment_user_message__single_query_single_retriever() {
Executor executor = mock(Executor.class);

RetrievalAugmentor retrievalAugmentor = DefaultRetrievalAugmentor.builder()
.queryTransformer(queryTransformer)
.queryRouter(queryRouter)
.contentAggregator(contentAggregator)
.contentInjector(contentInjector)
.executor(executor)
.build();
.queryTransformer(queryTransformer)
.queryRouter(queryRouter)
.contentAggregator(contentAggregator)
.contentInjector(contentInjector)
.executor(executor)
.build();

UserMessage userMessage = UserMessage.from("query");

Expand All @@ -252,7 +274,7 @@ void should_augment_user_message__single_query_single_retriever() {

// then
assertThat(augmented.singleText()).isEqualTo(
"""
"""
query
content 1
content 2"""
Expand Down Expand Up @@ -289,9 +311,9 @@ void should_not_augment_when_router_does_not_return_retrievers(Executor executor
QueryRouter queryRouter = spy(new TestQueryRouter(retrievers));

RetrievalAugmentor retrievalAugmentor = DefaultRetrievalAugmentor.builder()
.queryRouter(queryRouter)
.executor(executor)
.build();
.queryRouter(queryRouter)
.executor(executor)
.build();

UserMessage userMessage = UserMessage.from("query");

Expand All @@ -309,14 +331,14 @@ void should_not_augment_when_router_does_not_return_retrievers(Executor executor

static Stream<Executor> executors() {
return Stream.<Executor>builder()
.add(Executors.newCachedThreadPool())
.add(Executors.newFixedThreadPool(1))
.add(Executors.newFixedThreadPool(2))
.add(Executors.newFixedThreadPool(3))
.add(Executors.newFixedThreadPool(4))
.add(Runnable::run) // same thread executor
.add(null) // to use default Executor in DefaultRetrievalAugmentor
.build();
.add(Executors.newCachedThreadPool())
.add(Executors.newFixedThreadPool(1))
.add(Executors.newFixedThreadPool(2))
.add(Executors.newFixedThreadPool(3))
.add(Executors.newFixedThreadPool(4))
.add(Runnable::run) // same thread executor
.add(null) // to use default Executor in DefaultRetrievalAugmentor
.build();
}

static class TestQueryTransformer implements QueryTransformer {
Expand Down Expand Up @@ -366,10 +388,10 @@ static class TestContentAggregator implements ContentAggregator {
@Override
public List<Content> aggregate(Map<Query, Collection<List<Content>>> queryToContents) {
return queryToContents.values()
.stream()
.flatMap(Collection::stream)
.flatMap(List::stream)
.collect(toList());
.stream()
.flatMap(Collection::stream)
.flatMap(List::stream)
.collect(toList());
}
}

Expand All @@ -378,8 +400,8 @@ static class TestContentInjector implements ContentInjector {
@Override
public UserMessage inject(List<Content> contents, UserMessage userMessage) {
String joinedContents = contents.stream()
.map(it -> it.textSegment().text())
.collect(joining("\n"));
.map(it -> it.textSegment().text())
.collect(joining("\n"));
return UserMessage.from(userMessage.text() + "\n" + joinedContents);
}
}
Expand Down
Loading

0 comments on commit 9178367

Please sign in to comment.