From 362653cb475163b5f9fa8cfbfef326735643e852 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 | 93 ------------------- JShellAPI/build.gradle | 7 +- .../org/togetherjava/jshellapi/Config.java | 5 +- .../jshellapi/rest/ApiEndpoints.java | 5 +- .../jshellapi/service/DockerService.java | 11 +-- ...itional-spring-configuration-metadata.json | 2 +- JShellAPI/src/main/resources/application.yaml | 4 +- .../jshellapi/JShellApiTests.java | 52 ++++------- README.md | 8 ++ 9 files changed, 43 insertions(+), 144 deletions(-) diff --git a/JShellAPI/README.MD b/JShellAPI/README.MD index f032214..72938e2 100644 --- a/JShellAPI/README.MD +++ b/JShellAPI/README.MD @@ -102,96 +102,3 @@ The maximum ram allocated per container, in megabytes. The cpu configuration of each container, see [--cpus option of docker](https://docs.docker.com/config/containers/resource_constraints/#cpu). ### 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. - -### 2. Gradle Configuration for Tests - -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. -- **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/rest/ApiEndpoints.java b/JShellAPI/src/main/java/org/togetherjava/jshellapi/rest/ApiEndpoints.java index 889014f..fa068d6 100644 --- a/JShellAPI/src/main/java/org/togetherjava/jshellapi/rest/ApiEndpoints.java +++ b/JShellAPI/src/main/java/org/togetherjava/jshellapi/rest/ApiEndpoints.java @@ -1,10 +1,7 @@ package org.togetherjava.jshellapi.rest; /** - * This class holds endpoints mentioned in controllers. The main objective is to keep endpoints - * synchronized with testing classes. - * - * @author Firas Regaieg + * Holds endpoints mentioned in controllers. */ 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..e2c694b 100644 --- a/JShellAPI/src/test/java/org/togetherjava/jshellapi/JShellApiTests.java +++ b/JShellAPI/src/test/java/org/togetherjava/jshellapi/JShellApiTests.java @@ -19,10 +19,7 @@ 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 +28,37 @@ 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 - - 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 snippet execution + // -- first code snippet eval + executeCodeEvalTest(testEvalId, "int a = 2+2;", 1, "4"); - final String secondCodeExpression = "a * 2"; - - final JShellSnippetResult secondCodeSnippet = new JShellSnippetResult(SnippetStatus.VALID, - SnippetType.ADDITION, 2, secondCodeExpression, "8"); + // -- second code snippet eval + executeCodeEvalTest(testEvalId, "a * 2", 2, "8"); + } - final JShellResult secondCodeExpectedResult = - getJShellResultDefaultInstance(secondCodeSnippet); + private void executeCodeEvalTest(String evalId, String codeSnippet, int expectedId, + String expectedResult) { + final JShellSnippetResult jshellCodeSnippet = new JShellSnippetResult(SnippetStatus.VALID, + SnippetType.ADDITION, expectedId, codeSnippet, expectedResult); - assertThat(testEval(testEvalId, secondCodeExpression)).isEqualTo(secondCodeExpectedResult); + assertThat(testEval(evalId, codeSnippet)) + .isEqualTo(new JShellResult(List.of(jshellCodeSnippet), null, false, "")); } 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, ""); } } diff --git a/README.md b/README.md index d5299a6..b9d1425 100644 --- a/README.md +++ b/README.md @@ -47,3 +47,11 @@ JShellAPI is a REST API, and whenever some code is received, it will create a se ## How to use JShellApi ? See [JShellAPI README](JShellAPI/README.MD) + +## Testing +**JShellWrapper** Docker image lifecycle is handled during tests. + +The image name is injected from the root [build.gradle](./build.gradle) file, to JShellAPI project's [build.gradle](./JShellAPI/build.gradle) file and also to its [application.yaml](./JShellAPI/src/main/resources/application.yaml). + +- 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.