Conversation
…l need to be done... Also, sending files might be a too large a task.
|
Warning Rate limit exceeded@bamsemats has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 39 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughIntroduces a JavaFX chat application with file-sharing support. Adds new model classes (ChatMessage, EnvLoader), environment configuration loading, network messaging via HTTP, async message handling, dynamic UI filtering, file selection and upload capabilities, comprehensive test coverage, and updated build configuration with new dependencies. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Controller as HelloController
participant Model as HelloModel
participant HTTP as HTTP Client
participant NTFY as NTFY Server
User->>Controller: Type message + Click send()
Controller->>Model: sendMessage(username, text)
activate Model
Model->>Model: Build JSON payload
Model->>HTTP: POST with username, message, timestamp
activate HTTP
HTTP->>NTFY: Send to /topic
NTFY-->>HTTP: 200 OK
deactivate HTTP
deactivate Model
Model->>Controller: listenForMessages(callback)
activate Model
loop Listen for incoming
Model->>NTFY: GET /topic (streaming)
NTFY-->>Model: Message envelope
Model->>Model: parseEnvelopeToChatMessage()
Model->>Controller: callback(ChatMessage)
Controller->>Controller: Add to ObservableList
Controller->>Controller: Re-render ListView
end
sequenceDiagram
actor User
participant Controller as HelloController
participant FileChooser as FileChooser
participant Model as HelloModel
participant HTTP as HTTP Client
participant NTFY as NTFY Server
User->>Controller: Click attachButton
Controller->>FileChooser: Show file picker
FileChooser-->>Controller: Selected file path
Controller->>Model: onAttachFile() internally calls sendFile(File)
activate Model
Model->>HTTP: POST multipart/form-data (file + metadata)
activate HTTP
HTTP->>NTFY: Upload to /topic
NTFY-->>HTTP: URL response
deactivate HTTP
Model->>Model: Construct ChatMessage with fileUrl & mimeType
Model->>Controller: callback with file-attached message
deactivate Model
Controller->>Controller: Add to ObservableList + Render
User->>Controller: Click inline image/link in chat
Controller->>HostServices: openDocument(fileUrl)
HostServices->>User: Open in browser
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (14)
javafx.iml (1)
1-8: Consider excluding IDE-specific files from version control.IntelliJ IDEA module files (
.iml) are typically IDE-specific and can vary between team members' configurations. Consider adding*.imlto.gitignoreto prevent potential merge conflicts.src/main/java/com/example/EnvLoader.java (2)
13-27: Consider more robust .env file handling.The current implementation has a few limitations:
Hardcoded file path: The
.envfile is read from the current working directory, which may not be consistent across different execution contexts (IDE, command line, JAR execution). Consider using a path relative to the classpath or project root.No support for quoted values: The parser doesn't handle quoted values like
KEY="value with spaces"orKEY='value', which is common in.envfiles. Values with special characters or spaces won't be parsed correctly.Silent failure: IOException only logs a warning. If the
.envfile is required, consider failing fast with a proper exception.Example improvement for quoted value support:
String[] parts = line.split("=", 2); if (parts.length == 2) { - env.put(parts[0].trim(), parts[1].trim()); + String value = parts[1].trim(); + // Remove surrounding quotes if present + if ((value.startsWith("\"") && value.endsWith("\"")) || + (value.startsWith("'") && value.endsWith("'"))) { + value = value.substring(1, value.length() - 1); + } + env.put(parts[0].trim(), value); }
29-31: Consider adding overload with default value support.The
get()method returnsnullwhen a key is not found, which can lead toNullPointerExceptions. Consider adding an overload that accepts a default value.Example:
public static String get(String key) { return env.get(key); } public static String get(String key, String defaultValue) { return env.getOrDefault(key, defaultValue); }src/main/java/com/example/HelloFX.java (1)
11-15: Make the static field private for better encapsulation.The
hostServicesfield is declared aspublic static, which allows direct mutation from anywhere. Consider making itprivateto enforce access through the getter method.Apply this diff:
- public static HostServices hostServices; + private static HostServices hostServices; public static HostServices hostServices() { return hostServices; }src/main/resources/com/example/hello-view.fxml (2)
19-22: Consider adding accessibility improvements.The username field and controls lack accessibility features:
- Prompt text: The
usernameFieldshould have apromptTextattribute to guide users.- Mnemonics: Consider adding keyboard shortcuts (e.g.,
_Username:label text) for keyboard-only navigation.- Tooltips: Controls like the attach button could benefit from tooltips.
Example:
<TextField fx:id="usernameField" prefWidth="200" promptText="Enter your username" />
48-48: Add prompt text to input field.The message input field lacks a
promptTextattribute to guide users on what to type.Example:
<TextField fx:id="inputField" onAction="#onSend" promptText="Type a message..." HBox.hgrow="ALWAYS" />src/main/java/com/example/HelloModel.java (4)
22-27: Constructor’s hard dependency on.envis fine for app, but awkward for tests and reuseFail‑fast on missing
NTFY_URLis reasonable for the running app, but it forces any consumer (including tests and tools) to have a valid.env. A small overload likeHelloModel(String topicUrl)or a factory would make the model easier to test and reuse while keeping the default constructor for production.
37-106:sendFileis already asynchronous; document this and avoid nesting extra threads elsewhere
sendFiledoes its ownnew Thread(...).start(), which is good for keeping I/O off the FX thread, but it means callers should treat it as fire‑and‑forget. Right nowHelloController.onAttachFilewrapssendFilein yet anothernew Thread, which is redundant and makes debugging harder.I’d recommend:
- Documenting in Javadoc that
sendFileis asynchronous, and- Calling it directly from the controller (no extra thread) to avoid unnecessary thread nesting.
138-214: History/listen threads lack lifecycle management (optional improvement)
loadHistoryandlistenForMessagesspawn background threads that run until the HTTP stream ends (or errors) with no way to cancel from the outside. For a simple demo this is acceptable, but in a longer‑lived app you’ll likely want some lifecycle control (interrupt flags, closing the connection on shutdown, or using anExecutorService).Not urgent for a sample/chat toy, but worth keeping in mind if you build on this.
218-253: Envelope parsing is robust but swallows errors; consider logging more structurally
parseEnvelopeToChatMessagedefensively checks themessageshape and wraps JSON parsing in atry/catch, returningnullon any error, which keeps the stream loops resilient. The trade‑off is that you only get stacktraces to stderr, which can be noisy and hard to correlate.If you keep evolving this, consider:
- Logging parse failures with a short summary including the
idvalue, and- Maybe returning a sentinel
ChatMessagefor clearly invalid payloads if you ever want to surface them in the UI.Behavior as‑is is fine for now.
src/main/java/com/example/HelloController.java (4)
49-84: Handle image‑load failures by resetting the cell graphicIf
new Image(msg.getFileUrl(), true)or any of the image code throws, the catch block only prints the stacktrace and never callssetGraphic(...), which can leave stale content from a previous cell reuse. It’s safer to always set some graphic (even a text‑only fallback) in both success and failure paths.Example tweak inside the
catch:- if (msg.getMimeType() != null && msg.getMimeType().startsWith("image/")) { + if (msg.getMimeType() != null && msg.getMimeType().startsWith("image/")) { try { ImageView imageView = new ImageView(new Image(msg.getFileUrl(), true)); imageView.setFitWidth(200); imageView.setPreserveRatio(true); Text messageText = new Text(msg.getMessage() + "\n"); messageText.setStyle("-fx-font-size: 14px;"); setGraphic(new TextFlow(user, time, messageText, imageView)); - } catch (Exception e) { e.printStackTrace(); } + } catch (Exception e) { + e.printStackTrace(); + Text messageText = new Text(msg.getMessage() + "\n"); + messageText.setStyle("-fx-font-size: 14px;"); + setGraphic(new TextFlow(user, time, messageText)); + }
93-98: Filter predicate is simple and correct, but won’t retroactively tag old usernamesThe predicate
msg -> !hideMine || !current.equals(msg.getUsername())cleanly hides only messages with the current username; messages sent before a username change won’t be treated as “mine” anymore. That’s probably acceptable, but keep it in mind if you later introduce a stable user id.
100-111:onSendthreading approach is reasonable; consider centralizing async behavior in the model laterSpawning a short‑lived thread per send avoids blocking the FX thread on network I/O, which is fine for a small app. Long‑term you might want
HelloModelitself to expose async APIs (or use anExecutorService) so controllers don’t manage threads directly.
113-119: Avoid double‑threading for file send; callsendFiledirectly here
onAttachFilestarts a new thread and then callsmodel.sendFile, which itself creates another thread for the actual upload. The outer thread does almost nothing except immediately spawn the inner one.You can simplify to a single background thread by removing the extra
new Thread:- File file = fileChooser.showOpenDialog(chatList.getScene().getWindow()); - if (file != null) new Thread(() -> model.sendFile(getCurrentUsername(), file)).start(); + File file = fileChooser.showOpenDialog(chatList.getScene().getWindow()); + if (file != null) { + model.sendFile(getCurrentUsername(), file); // already async + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
src/main/resources/background.pngis excluded by!**/*.pngsrc/main/resources/lantern.pngis excluded by!**/*.pngsrc/main/resources/lantern1.pngis excluded by!**/*.pngsrc/main/resources/lantern2.pngis excluded by!**/*.pngsrc/main/resources/lantern3.pngis excluded by!**/*.pngsrc/main/resources/lantern4.pngis excluded by!**/*.png
📒 Files selected for processing (12)
.gitignore(1 hunks)javafx.iml(1 hunks)pom.xml(5 hunks)src/main/java/com/example/ChatMessage.java(1 hunks)src/main/java/com/example/EnvLoader.java(1 hunks)src/main/java/com/example/HelloController.java(1 hunks)src/main/java/com/example/HelloFX.java(1 hunks)src/main/java/com/example/HelloModel.java(1 hunks)src/main/java/module-info.java(1 hunks)src/main/resources/com/example/hello-view.fxml(1 hunks)src/main/resources/css/style.css(1 hunks)src/test/java/com/example/HelloModelTest.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/main/java/com/example/HelloModel.java (1)
src/main/java/com/example/EnvLoader.java (1)
EnvLoader(9-32)
src/main/java/com/example/HelloController.java (1)
src/main/java/com/example/HelloFX.java (1)
HelloFX(10-32)
🪛 Biome (2.1.2)
src/main/resources/css/style.css
[error] 2-2: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 3-3: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 7-7: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 11-11: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 12-12: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 16-16: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 17-17: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 18-18: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 18-18: Unexpected unknown function: derive
Use a known function instead.
See MDN web docs for more details.
(lint/correctness/noUnknownFunction)
[error] 22-22: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
🔇 Additional comments (6)
.gitignore (1)
3-3: LGTM!Adding
.envto.gitignoreis appropriate to prevent environment configuration files from being committed to version control.src/main/java/module-info.java (1)
4-5: LGTM!The new module requirements for
org.jsonandjava.net.httpalign with the added Maven dependencies and support the chat application's JSON processing and HTTP communication capabilities.src/main/resources/css/style.css (1)
1-23: Static analysis hints are false positives.The static analysis tool (Biome) is flagging JavaFX-specific CSS properties and functions as unknown because it's designed for web CSS, not JavaFX CSS. All properties (prefixed with
-fx-) and thederive()function are valid JavaFX CSS extensions and are correctly used in this stylesheet.src/test/java/com/example/HelloModelTest.java (1)
70-94: JSON envelope parsing tests look good and matchparseEnvelopeToChatMessagesemanticsThese two tests nicely cover the “JSON message vs plain text” envelope behavior and verify that a proper
ChatMessageis created only for the JSON case. This is a solid regression net around the parsing logic.src/main/java/com/example/HelloController.java (1)
89-91: Good use ofPlatform.runLaterto marshal model callbacks onto the FX threadWiring
loadHistory/listenForMessagesintomasterListthroughPlatform.runLateris the right pattern and keeps UI updates thread‑safe.src/main/java/com/example/ChatMessage.java (1)
3-34: ImmutableChatMessagevalue object looks solidFields are
final, constructors cover both “with attachment” and “text‑only” cases, and getters are straightforward. This is a clean, minimal data carrier for the rest of the app.
| <dependency> | ||
| <groupId>org.json</groupId> | ||
| <artifactId>json</artifactId> | ||
| <version>20240303</version> | ||
| </dependency> | ||
|
|
||
| <dependency> | ||
| <groupId>org.apache.httpcomponents.client5</groupId> | ||
| <artifactId>httpclient5</artifactId> | ||
| <version>${httpclient.version}</version> | ||
| </dependency> |
There was a problem hiding this comment.
🧩 Analysis chain
Verify dependency versions and security advisories.
Since the project uses fixed versions for org.json (20240303, from March 2024) and httpclient5 (5.3.1), please verify these are current and check for any security advisories.
Run the following script to check for the latest versions and security advisories:
🏁 Script executed:
#!/bin/bash
# Check latest versions from Maven Central
echo "=== Checking org.json:json latest version ==="
curl -s "https://search.maven.org/solrsearch/select?q=g:org.json+AND+a:json&rows=1&wt=json" | jq -r '.response.docs[0].latestVersion'
echo "=== Checking httpclient5 latest version ==="
curl -s "https://search.maven.org/solrsearch/select?q=g:org.apache.httpcomponents.client5+AND+a:httpclient5&rows=1&wt=json" | jq -r '.response.docs[0].latestVersion'
echo "=== Checking for security advisories ==="
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: MAVEN, package: "org.json:json") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: MAVEN, package: "org.apache.httpcomponents.client5:httpclient5") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'Length of output: 1094
Update outdated dependencies in pom.xml.
HttpClient 5.5.x is stable and production ready, and actively maintained, while the current version (5.3.1) is from an older release series. Latest stable httpclient5 is 5.5.1.
org.json latest version is 20250517 (May 2025), but the pom.xml uses 20240303 (March 2024)—17 months behind. Both current versions lack recent security patches and improvements.
Update:
org.json:jsonfrom20240303to20250517httpclient5from5.3.1to5.5.1
🤖 Prompt for AI Agents
In pom.xml around lines 56 to 66, the project is using outdated dependency
versions: org.json:json is set to 20240303 and httpclient5 is resolved to
${httpclient.version} which currently points to 5.3.1; update org.json:json to
version 20250517 and update httpclient5 to 5.5.1 (either by changing the
explicit <version> for the httpclient5 dependency to 5.5.1 or by updating the
httpclient.version property in the pom.xml to 5.5.1) so the POM uses
org.json:json:20250517 and org.apache.httpcomponents.client5:httpclient5:5.5.1.
| Parent root = loader.load(); | ||
| Scene scene = new Scene(root, 720, 520); | ||
| scene.getStylesheets().add(getClass().getResource("/css/style.css").toExternalForm()); | ||
| stage.setTitle("JavaFX NTFY Chat — mats_notiser"); |
There was a problem hiding this comment.
Remove hardcoded username from window title.
The window title includes a hardcoded username "mats_notiser", which appears to be developer-specific. Since the UI includes a usernameField, consider either:
- Using a generic title without a username
- Dynamically updating the title when the username changes
Example for a generic title:
- stage.setTitle("JavaFX NTFY Chat — mats_notiser");
+ stage.setTitle("JavaFX NTFY Chat");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| stage.setTitle("JavaFX NTFY Chat — mats_notiser"); | |
| stage.setTitle("JavaFX NTFY Chat"); |
🤖 Prompt for AI Agents
In src/main/java/com/example/HelloFX.java around line 24, remove the hardcoded
username in the window title ("mats_notiser") and either set a generic title
(e.g., "JavaFX NTFY Chat") or make the title dynamic by updating/binding it to
the usernameField so it reflects the entered username; implement this by
replacing the literal title string with a generic one or by adding a
listener/binding on usernameField.textProperty() to update stage.setTitle(...)
when the username changes (use a fallback when the field is empty).
| <?import javafx.scene.control.*?> | ||
| <?import javafx.scene.layout.*?> | ||
|
|
||
| <BorderPane xmlns:fx="http://javafx.com/fxml/1" xmlns="http://javafx.com/javafx/17.0.12" fx:controller="com.example.HelloController"> |
There was a problem hiding this comment.
🧩 Analysis chain
Verify controller implementation matches FXML bindings.
The FXML references HelloController with multiple fx:id bindings and action handlers. Please ensure the controller has:
@FXMLannotated fields:usernameField,hideMyMessagesCheck,attachButton,chatList,uploadProgress,statusLabel,inputField,sendButton- Action handler methods:
onAttachFile()andonSend() - Proper initialization logic in the
initialize()method
Run the following script to verify the controller implementation:
🏁 Script executed:
#!/bin/bash
# Verify HelloController exists and has required fields and methods
echo "=== Checking for HelloController class ==="
rg -n "class HelloController" --type java
echo "=== Checking for @FXML fields ==="
rg -n "@FXML" --type java -A 1 -g '**/HelloController.java'
echo "=== Checking for action handlers ==="
rg -n "(onAttachFile|onSend)" --type java -g '**/HelloController.java'
echo "=== Checking for initialize method ==="
rg -n "void initialize\(" --type java -g '**/HelloController.java'Length of output: 1253
🏁 Script executed:
#!/bin/bash
# Get complete list of @FXML fields and verify method signatures
echo "=== All @FXML fields in HelloController ==="
rg -n "@FXML" --type java -A 1 src/main/java/com/example/HelloController.java
echo "=== Action handler signatures ==="
rg -nP "(@FXML\s+)?private void (onAttachFile|onSend)" --type java -A 1 src/main/java/com/example/HelloController.java
echo "=== Check FXML file for all fx:id and fx:controller references ==="
grep -n "fx:id\|fx:controller" src/main/resources/com/example/hello-view.fxmlLength of output: 1387
Missing @FXML field bindings in HelloController will cause runtime failures.
The FXML file defines 8 fx:id bindings, but the controller only implements 4 @FXML fields. Missing fields that must be added to src/main/java/com/example/HelloController.java:
attachButton(Button)uploadProgress(ProgressBar)statusLabel(Label)sendButton(Button)
The controller has correct @FXML annotations for usernameField, hideMyMessagesCheck, chatList, and inputField. The action handlers onSend() and onAttachFile() are properly defined. Add the missing field declarations to prevent JavaFX injection errors.
🤖 Prompt for AI Agents
In src/main/java/com/example/HelloController.java (corresponding to
src/main/resources/com/example/hello-view.fxml around line 7), the controller is
missing four @FXML field declarations that the FXML references, causing JavaFX
injection failures at runtime; add private fields annotated with @FXML for
attachButton (javafx.scene.control.Button), uploadProgress
(javafx.scene.control.ProgressBar), statusLabel (javafx.scene.control.Label),
and sendButton (javafx.scene.control.Button) to the HelloController class
alongside the existing @FXML fields so the FXML loader can inject those UI
controls.
| @Test | ||
| void testLoadHistoryCallbackCalled() { | ||
| AtomicBoolean called = new AtomicBoolean(false); | ||
| model.loadHistory(msg -> called.set(true)); | ||
| assertDoesNotThrow(() -> model.loadHistory(msg -> {})); | ||
| } | ||
|
|
||
| @Test | ||
| void testListenForMessagesCallbackCalled() { | ||
| AtomicReference<ChatMessage> ref = new AtomicReference<>(); | ||
| model.listenForMessages(ref::set); | ||
| assertDoesNotThrow(() -> model.listenForMessages(msg -> {})); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
loadHistory/listenForMessages tests don’t assert callbacks and still hit real network
Both testLoadHistoryCallbackCalled and testListenForMessagesCallbackCalled only assert “no exception thrown” and never check called.get() / ref.get(), so they do not actually verify that callbacks are invoked. On top of that, they call the real loadHistory/listenForMessages, which spin up threads and try to talk to whatever NTFY_URL points at, making tests slow and brittle.
Consider either:
- Overriding
loadHistory/listenForMessagesin the anonymous subclass to invoke the callback immediately without networking; or - Using a
CountDownLatchwith a short timeout and a stubbedHelloModelthat doesn’t talk to the network.
Also rename the tests (or tighten the assertions) so names match what is actually verified.
🏁 Script executed:
#!/bin/bash
# Examine the test file to see full context and class setup
cat src/test/java/com/example/HelloModelTest.javaLength of output: 3425
🏁 Script executed:
#!/bin/bash
# Search for HelloModel class definition and implementation
fd -t f HelloModel.java | head -5 | xargs -I {} cat {}Length of output: 10595
🏁 Script executed:
#!/bin/bash
# Look for NTFY_URL and network-related code
rg -n "NTFY_URL|loadHistory|listenForMessages" --type=java -B2 -A5Length of output: 4123
🏁 Script executed:
#!/bin/bash
# Check if there are any mock frameworks being used
rg -n "@Mock|@Spy|mockito|when\(|thenReturn" --type=javaLength of output: 0
Tests don't verify callback invocation and make real network requests
Both testLoadHistoryCallbackCalled and testListenForMessagesCallbackCalled have two problems:
-
No callback verification: The tests initialize
calledandrefbut never assert on them. AddingassertTrue(called.get())andassertNotNull(ref.get())after the first call would catch missing callbacks. -
Real network calls:
loadHistoryandlistenForMessagesare not overridden in the test's anonymousHelloModelsubclass, so they execute the real implementations that spawn threads and make HTTP requests toTOPIC_URL, making tests slow and brittle.
Fix by either:
- Overriding both methods in the anonymous subclass to invoke callbacks immediately without networking, or
- Using a
CountDownLatchwith a timeout to verify async callback invocation on a stubbed model
Also rename tests (or add proper assertions) to match what is actually verified.
🤖 Prompt for AI Agents
In src/test/java/com/example/HelloModelTest.java around lines 97 to 109, the two
tests call model.loadHistory and model.listenForMessages but neither assert the
callbacks were invoked and they call the real implementations which perform
network work; update the tests to (1) either override loadHistory and
listenForMessages in the test’s HelloModel stub to synchronously invoke the
provided callbacks (so no network/threads run) and then add assertions
assertTrue(called.get()) and assertNotNull(ref.get()) after the call, or (2)
replace with a CountDownLatch per test that the overridden/stubbed async
callback counts down and use await with a timeout, then assert the latch reached
zero; also rename or update test names to reflect the verified behavior.
2 participants
Wanna try a pull request..?
Summary by CodeRabbit
New Features
Chores