test: Graalvm smoke test#379
Closed
gastonfournier wants to merge 5 commits into
Closed
Conversation
As reported in #377 - getContent() when building with GraalVM uses serviceloaders to locate the correct content processor. But we've already checked that we don't have a gzip stream, so we're trusting our upstream to return application/json and do not need a content processor. This saves us a cast from content to InputStream as well. fixes: #377
gastonfournier
force-pushed
the
graalvm-smoke-test
branch
from
3beef1f to
89aca5f
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a GraalVM native-image smoke test to validate HttpFeatureFetcher works in a native binary, and adjusts a small input-stream access detail.
Changes:
- Introduces a standalone smoke-test main class that spins up a local socket server and validates feature fetching.
- Updates
HttpFeatureFetcherto usegetInputStream()instead of castinggetContent()toInputStream. - Adds a GitHub Actions job to build and run the smoke test as a GraalVM native image.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/test/java/io/getunleash/repository/GraalVmHttpFeatureFetcherSmoke.java | Adds a native-image-friendly smoke test that exercises HttpFeatureFetcher end-to-end. |
| src/main/java/io/getunleash/repository/HttpFeatureFetcher.java | Switches to request.getInputStream() for reading the response body. |
| pom.xml | Fixes project version string and applies formatting changes. |
| .github/workflows/pull_requests.yml | Adds a CI job to build and execute the GraalVM native-image smoke test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| with: | ||
| distribution: graalvm | ||
| java-version: "21" | ||
| cache: maven |
Member
There was a problem hiding this comment.
It does look like this might be correct, components: 'native-image' might be needed here.
Comment on lines
+33
to
+35
| graalvm-native-image: | ||
| runs-on: ubuntu-latest | ||
| steps: |
| <groupId>io.getunleash</groupId> | ||
| <artifactId>unleash-client-java</artifactId> | ||
| <version>12.2.3-SNAPSHOT</version> | ||
| <version>12.2.2-SNAPSHOT</version> |
| socket.getInputStream(), StandardCharsets.US_ASCII)); | ||
| OutputStream output = socket.getOutputStream()) { | ||
| String line; | ||
| while ((line = reader.readLine()) != null && !line.isEmpty()) {} |
Comment on lines
+68
to
+70
| } catch (Exception e) { | ||
| throw new RuntimeException(e); | ||
| } |
Collaborator
Coverage Report for CI Build 26279738811Coverage increased (+0.2%) to 79.869%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
Contributor
Author
|
This doesn't seem to catch any problem so I will just close this idea for now |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Attempt to have a GraalVM validation step