From 4f4ec50355419e44c54f464e004a1d0bff891497 Mon Sep 17 00:00:00 2001 From: Firas RG Date: Fri, 20 Sep 2024 20:45:54 +0100 Subject: [PATCH] [Testing][JShellAPI] apply pr review fixes --- JShellAPI/README.MD | 109 +++--------------- JShellAPI/build.gradle | 7 +- .../org/togetherjava/jshellapi/Config.java | 5 +- .../jshellapi/dto/JShellResult.java | 4 + .../jshellapi/rest/ApiEndpoints.java | 1 - .../jshellapi/service/DockerService.java | 11 +- ...itional-spring-configuration-metadata.json | 2 +- JShellAPI/src/main/resources/application.yaml | 4 +- .../jshellapi/JShellApiTests.java | 42 +++---- 9 files changed, 46 insertions(+), 139 deletions(-) diff --git a/JShellAPI/README.MD b/JShellAPI/README.MD index f032214..b89b139 100644 --- a/JShellAPI/README.MD +++ b/JShellAPI/README.MD @@ -87,111 +87,28 @@ The following startup scripts id can be used : - EMPTY : no startup script - CUSTOM_DEFAULT : contains basic imports, print methods and range method ## Configuration + +### Properties Properties can be defined in resources/application.properties -### jshellapi.regularSessionTimeoutSeconds +#### jshellapi.regularSessionTimeoutSeconds The timeout of a regular session, in seconds, see [Session timeout](#Session-timeout). -### jshellapi.oneTimeSessionTimeoutSeconds +#### jshellapi.oneTimeSessionTimeoutSeconds The timeout of a one-time session, in seconds, see [One-time session timeout](#One-time-session-timeout). -### jshellapi.evalTimeoutSeconds +#### jshellapi.evalTimeoutSeconds The timeout of an evaluation, in seconds, see [Eval timeout](#Eval-timeout) -### jshellapi.maxAliveSessions +#### jshellapi.maxAliveSessions The maximum number of alive sessions, see [Errors](#Errors) -### jshellapi.dockerMaxRamMegaBytes +#### jshellapi.dockerMaxRamMegaBytes The maximum ram allocated per container, in megabytes. -### jshellapi.dockerCPUsUsage +#### jshellapi.dockerCPUsUsage The cpu configuration of each container, see [--cpus option of docker](https://docs.docker.com/config/containers/resource_constraints/#cpu). -### jshellapi.schedulerSessionKillScanRate +#### jshellapi.schedulerSessionKillScanRate The rate at which the session killer will check and delete session, in seconds, see [Session timeout](#Session-timeout). -## Testing - -> The work on testing was made in collaboration with [Alathreon](https://github.com/Alathreon) and [Wazei](https://github.com/tj-wazei). I'd like thank both of them for their trust. - FirasRG - -This section outlines the work done to set up the first integration test that evaluates Java code by running it in a [Docker](https://www.docker.com/get-started/) container. The test ensures that the [Eval endpoint](#eval) can execute code within the containerized environment of [**JShellWrapper**](../JShellWrapper). - -### Usage - -```java -@ContextConfiguration(classes = Main.class) -@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) -public class JShellApiTests { - - @Autowired - private WebTestClient webTestClient; - - @Test - @DisplayName("When posting code snippet, evaluate it then returns successfully result") - public void evaluateCodeSnippetTest() { - - final String testEvalId = "test"; - - final String firstCodeExpression = "int a = 2+2;"; - - final JShellSnippetResult firstCodeSnippet = new JShellSnippetResult(SnippetStatus.VALID, SnippetType.ADDITION, 1, firstCodeExpression, "4"); - - final JShellResult firstCodeExpectedResult = getJShellResultDefaultInstance(firstCodeSnippet); - - assertThat(testEval(testEvalId, firstCodeExpression)).isEqualTo(firstCodeExpectedResult); - - // performing a second code execution test ... - } - // some methods ... -} -``` - -### 1. Java Test Setup - -The [@SpringBootTest](https://docs.spring.io/spring-boot/api/java/org/springframework/boot/test/context/SpringBootTest.html) and [@ContextConfiguration](https://docs.spring.io/spring-framework/reference/testing/annotations/integration-spring/annotation-contextconfiguration.html) annotations are needed to prepare the app to tests, like in a real scenario. - -NOTE: _Test classes must be located under `/src/test/java/{org.togetherjava.jshellapi}`._ - -- The test uses [WebTestClient](https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/test/web/reactive/server/WebTestClient.html) to make HTTP calls to the target endpoint. -- Multiple API calls are made within the test method, so a utility instance method was created for reuse. -- The test ensures that code is correctly evaluated inside the **JShellWrapper** container. +### Testing +**JShellWrapper** Docker image lifecycle is handled during tests. -### 2. Gradle Configuration for Tests +The image name is injected from the root [build.gradle](../build.gradle) file, to this project's [build.gradle](build.gradle) file and also to [application.yaml](src/main/resources/application.yaml). -The `build.gradle` of this project has been updated to handle **JShellWrapper** Docker image lifecycle during tests. - -- **JShellWrapper Image Name**: the image name is injected from the root [build.gradle](../build.gradle) file, to this project's [build.gradle](build.gradle) file and also to [application.yaml](src/main/resources/application.yaml)! -- **JShellWrapper Docker Image**: The image is built before the tests run. +- JShellWrapper Docker Image is built before the tests run. - **Container & Cleanup**: After the tests finish, the container and image are removed to ensure a clean environment. - -```groovy -def jshellWrapperImageName = rootProject.ext.jShellWrapperImageName; - -processResources { - filesMatching('application.yaml') { - expand(jShellWrapperImageName: jshellWrapperImageName) - } -} - -def taskBuildDockerImage = tasks.register('buildDockerImage') { - group = 'docker' - description = 'builds jshellwrapper as docker image' - dependsOn project(':JShellWrapper').tasks.named('jibDockerBuild') -} - -def taskRemoveDockerImage = tasks.register('removeDockerImage', Exec) { - group = 'docker' - description = 'removes jshellwrapper image' - commandLine 'docker', 'rmi', '-f', jshellWrapperImageName -} - -test { - dependsOn taskBuildDockerImage - finalizedBy taskRemoveDockerImage -} -``` - -Below are the key dependencies that were added or modified in the `build.gradle` file of this project : - -```groovy -testImplementation('org.springframework.boot:spring-boot-starter-test') { - exclude group: 'ch.qos.logback', module: 'logback-classic' -} -testImplementation 'org.springframework.boot:spring-boot-starter-webflux' -``` - -- The `logback-classic` has been excluded because of an issue encountered when running tests. The issue is typically about a conflict between some dependencies (This solution has been brought based on [a _good_ answer on Stackoverflow](https://stackoverflow.com/a/42641450/10000150)) -- The `spring-boot-starter-webflux` was needed in order to be able to use **WebTestClient**. diff --git a/JShellAPI/build.gradle b/JShellAPI/build.gradle index 6ebd62c..b1bf927 100644 --- a/JShellAPI/build.gradle +++ b/JShellAPI/build.gradle @@ -14,6 +14,9 @@ dependencies { implementation 'com.github.docker-java:docker-java-core:3.3.6' testImplementation('org.springframework.boot:spring-boot-starter-test') { + // `logback-classic` has been excluded because of an issue encountered when running tests. + // It's about a conflict between some dependencies. + // The solution has been brought based on a good answer on Stackoverflow: https://stackoverflow.com/a/42641450/10000150 exclude group: 'ch.qos.logback', module: 'logback-classic' } testImplementation 'org.springframework.boot:spring-boot-starter-webflux' @@ -43,11 +46,13 @@ shadowJar { archiveVersion.set('') } +// -- Gradle testing configuration + def jshellWrapperImageName = rootProject.ext.jShellWrapperImageName; processResources { filesMatching('application.yaml') { - expand(jShellWrapperImageName: jshellWrapperImageName) + expand(jshellWrapperImageName: jshellWrapperImageName) } } diff --git a/JShellAPI/src/main/java/org/togetherjava/jshellapi/Config.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/Config.java index 4c337e9..55c2097 100644 --- a/JShellAPI/src/main/java/org/togetherjava/jshellapi/Config.java +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/Config.java @@ -2,13 +2,14 @@ import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.lang.Nullable; +import org.springframework.util.StringUtils; @ConfigurationProperties("jshellapi") public record Config(long regularSessionTimeoutSeconds, long oneTimeSessionTimeoutSeconds, long evalTimeoutSeconds, long evalTimeoutValidationLeeway, int sysOutCharLimit, long maxAliveSessions, int dockerMaxRamMegaBytes, double dockerCPUsUsage, @Nullable String dockerCPUSetCPUs, long schedulerSessionKillScanRateSeconds, - long dockerResponseTimeout, long dockerConnectionTimeout) { + long dockerResponseTimeout, long dockerConnectionTimeout, String jshellWrapperImageName) { public Config { if (regularSessionTimeoutSeconds <= 0) throw new IllegalArgumentException("Invalid value " + regularSessionTimeoutSeconds); @@ -35,5 +36,7 @@ public record Config(long regularSessionTimeoutSeconds, long oneTimeSessionTimeo throw new IllegalArgumentException("Invalid value " + dockerResponseTimeout); if (dockerConnectionTimeout <= 0) throw new IllegalArgumentException("Invalid value " + dockerConnectionTimeout); + if (!StringUtils.hasText(jshellWrapperImageName)) + throw new IllegalArgumentException("Invalid value " + jshellWrapperImageName); } } diff --git a/JShellAPI/src/main/java/org/togetherjava/jshellapi/dto/JShellResult.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/dto/JShellResult.java index 6ab2bc6..f7f4472 100644 --- a/JShellAPI/src/main/java/org/togetherjava/jshellapi/dto/JShellResult.java +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/dto/JShellResult.java @@ -9,4 +9,8 @@ public record JShellResult(List snippetsResults, public JShellResult { snippetsResults = List.copyOf(snippetsResults); } + + public JShellResult(JShellSnippetResult snippetsResultList) { + this(List.of(snippetsResultList), null, false, ""); + } } diff --git a/JShellAPI/src/main/java/org/togetherjava/jshellapi/rest/ApiEndpoints.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/rest/ApiEndpoints.java index 889014f..035f373 100644 --- a/JShellAPI/src/main/java/org/togetherjava/jshellapi/rest/ApiEndpoints.java +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/rest/ApiEndpoints.java @@ -4,7 +4,6 @@ * This class holds endpoints mentioned in controllers. The main objective is to keep endpoints * synchronized with testing classes. * - * @author Firas Regaieg */ public final class ApiEndpoints { private ApiEndpoints() {} diff --git a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java index fa1e171..cbf8541 100644 --- a/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/service/DockerService.java @@ -7,11 +7,9 @@ import com.github.dockerjava.core.DefaultDockerClientConfig; import com.github.dockerjava.core.DockerClientImpl; import com.github.dockerjava.httpclient5.ApacheDockerHttpClient; -import jakarta.el.PropertyNotFoundException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.DisposableBean; -import org.springframework.beans.factory.annotation.Value; import org.springframework.lang.Nullable; import org.springframework.stereotype.Service; @@ -31,8 +29,7 @@ public class DockerService implements DisposableBean { private final DockerClient client; - @Value("${jshell-wrapper.image-name}") - private String jshellWrapperImageName; + private final String jshellWrapperImageName; public DockerService(Config config) { DefaultDockerClientConfig clientConfig = @@ -44,6 +41,7 @@ public DockerService(Config config) { .connectionTimeout(Duration.ofSeconds(config.dockerConnectionTimeout())) .build(); this.client = DockerClientImpl.getInstance(clientConfig, httpClient); + this.jshellWrapperImageName = config.jshellWrapperImageName(); cleanupLeftovers(WORKER_UNIQUE_ID); } @@ -64,11 +62,8 @@ private void cleanupLeftovers(UUID currentId) { public String spawnContainer(long maxMemoryMegs, long cpus, @Nullable String cpuSetCpus, String name, Duration evalTimeout, long sysoutLimit) throws InterruptedException { - String imageName = Optional.ofNullable(this.jshellWrapperImageName) - .orElseThrow(() -> new PropertyNotFoundException( - "unable to find jshellWrapper image name property")); - String[] imageNameParts = imageName.split(":master"); + String[] imageNameParts = this.jshellWrapperImageName.split(":master"); if (imageNameParts.length != 1) { throw new IllegalArgumentException("invalid jshellWrapper image name"); diff --git a/JShellAPI/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/JShellAPI/src/main/resources/META-INF/additional-spring-configuration-metadata.json index 6c9bcc1..4699496 100644 --- a/JShellAPI/src/main/resources/META-INF/additional-spring-configuration-metadata.json +++ b/JShellAPI/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -1,7 +1,7 @@ { "properties": [ { - "name": "jshell-wrapper.image-name", + "name": "jshellapi.jshellwrapper-imageName", "type": "java.lang.String", "description": "JShellWrapper image name injected from the top-level gradle build file." } diff --git a/JShellAPI/src/main/resources/application.yaml b/JShellAPI/src/main/resources/application.yaml index 83a6e23..4b454fd 100644 --- a/JShellAPI/src/main/resources/application.yaml +++ b/JShellAPI/src/main/resources/application.yaml @@ -20,8 +20,8 @@ jshellapi: dockerResponseTimeout: 60 dockerConnectionTimeout: 60 -jshell-wrapper: - image-name: ${jShellWrapperImageName} + # JShellWrapper related + jshellwrapper-imageName: ${jshellWrapperImageName} server: error: diff --git a/JShellAPI/src/test/java/org/togetherjava/jshellapi/JShellApiTests.java b/JShellAPI/src/test/java/org/togetherjava/jshellapi/JShellApiTests.java index a56d505..c15f549 100644 --- a/JShellAPI/src/test/java/org/togetherjava/jshellapi/JShellApiTests.java +++ b/JShellAPI/src/test/java/org/togetherjava/jshellapi/JShellApiTests.java @@ -14,15 +14,11 @@ import org.togetherjava.jshellapi.rest.ApiEndpoints; import java.time.Duration; -import java.util.List; import static org.assertj.core.api.Assertions.assertThat; /** - * This class holds integration tests for JShellAPI. It depends on gradle building image task, fore - * more information check "test" section in gradle.build file. - * - * @author Firas Regaieg + * Integrates tests for JShellAPI. */ @ContextConfiguration(classes = Main.class) @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @@ -31,42 +27,38 @@ public class JShellApiTests { @Autowired private WebTestClient webTestClient; + @Autowired + private Config appConfig; + @Test - @DisplayName("When posting code snippet, evaluate it then returns successfully result") + @DisplayName("When posting code snippet, evaluate it then return successfully result") public void evaluateCodeSnippetTest() { final String testEvalId = "test"; - // -- performing a first code snippet execution + // -- first code snippet eval final String firstCodeExpression = "int a = 2+2;"; - final JShellSnippetResult firstCodeSnippet = new JShellSnippetResult(SnippetStatus.VALID, SnippetType.ADDITION, 1, firstCodeExpression, "4"); - final JShellResult firstCodeExpectedResult = - getJShellResultDefaultInstance(firstCodeSnippet); + assertThat(testEval(testEvalId, firstCodeExpression)) + .isEqualTo(new JShellResult(firstCodeSnippet)); - assertThat(testEval(testEvalId, firstCodeExpression)).isEqualTo(firstCodeExpectedResult); - - // -- performing a second code snippet execution + // -- second code snippet eval final String secondCodeExpression = "a * 2"; - final JShellSnippetResult secondCodeSnippet = new JShellSnippetResult(SnippetStatus.VALID, SnippetType.ADDITION, 2, secondCodeExpression, "8"); - - final JShellResult secondCodeExpectedResult = - getJShellResultDefaultInstance(secondCodeSnippet); - - assertThat(testEval(testEvalId, secondCodeExpression)).isEqualTo(secondCodeExpectedResult); + assertThat(testEval(testEvalId, secondCodeExpression)) + .isEqualTo(new JShellResult(secondCodeSnippet)); } private JShellResult testEval(String testEvalId, String codeInput) { final String endpoint = String.join("/", ApiEndpoints.BASE, ApiEndpoints.EVALUATE, testEvalId); - JShellResult result = this.webTestClient.mutate() - .responseTimeout(Duration.ofSeconds(6)) + return this.webTestClient.mutate() + .responseTimeout(Duration.ofSeconds(appConfig.evalTimeoutSeconds())) .build() .post() .uri(endpoint) @@ -78,13 +70,5 @@ private JShellResult testEval(String testEvalId, String codeInput) { .value((JShellResult evalResult) -> assertThat(evalResult).isNotNull()) .returnResult() .getResponseBody(); - - assertThat(result).isNotNull(); - - return result; - } - - private static JShellResult getJShellResultDefaultInstance(JShellSnippetResult snippetResult) { - return new JShellResult(List.of(snippetResult), null, false, ""); } }