-
Notifications
You must be signed in to change notification settings - Fork 0
Tests, mocks, test parallelism #107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the testing infrastructure and robustness of the Arc cache implementation. It introduces parallel test execution, expands test coverage with additional cases (e.g., null key handling, loader exception and retry, concurrent access with the same key), and enforces null-check invariants in the main cache code, along with updating module and dependency configurations.
- Enable JUnit test parallelism via properties.
- Add multiple tests addressing edge cases and concurrency behavior.
- Integrate null checks via a new Invariants class and update build/module configurations.
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
lib/src/test/resources/junit-platform.properties | Enabling parallel test execution |
lib/src/test/java/eu/aylett/arc/ArcTest.java | Addition of tests covering null keys, loader behavior, and concurrency |
lib/src/main/java/module-info.java | Updated module dependencies (added JetBrains annotations) |
lib/src/main/java/eu/aylett/arc/internal/Invariants.java | New utility class for null-checks |
lib/src/main/java/eu/aylett/arc/internal/Element.java | Updated get() method handling completed exceptions and weak values |
lib/src/main/java/eu/aylett/arc/Arc.java | Enforcing null-checks in constructor and get() method |
lib/build.gradle.kts | Added dependencies for JetBrains annotations and Mockito |
gradle/libs.versions.toml | Updated version references for JetBrains and Mockito |
gradle/gradle-daemon-jvm.properties | New Gradle daemon configuration properties |
@@ -269,7 +276,7 @@ record = recordedValues.stream().collect(Collectors.groupingBy((v) -> v, Collect | |||
.forEach(adjacentPair -> { | |||
var start = adjacentPair.getFirst(); | |||
var end = adjacentPair.getLast(); | |||
assertThat(end.toEpochMilli(), lessThan(start.plusSeconds(35).toEpochMilli())); | |||
assertThat(end.toEpochMilli(), lessThan(start.plusSeconds(55).toEpochMilli())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a comment explaining the rationale for increasing the timing threshold from 35 to 55 seconds in the eviction test to clarify the expected behavior.
Copilot uses AI. Check for mistakes.
var currentWeakValue = this.weakValue; | ||
if (currentWeakValue == null || !currentWeakValue.refersTo(v)) { | ||
this.weakValue = new WeakReference<>(v); | ||
if (currentValue == null || currentValue.isCompletedExceptionally()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment to clarify why elements with a completed exceptional future trigger a reload, to improve code maintainability and future understanding.
Copilot uses AI. Check for mistakes.
if (currentWeakValue != null) { | ||
var v = currentWeakValue.get(); | ||
if (v != null) { | ||
return (this.value = CompletableFuture.completedFuture(v)).copy(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a comment explaining the need to call copy() after creating the completed future to preserve immutability or prevent side effects.
Copilot uses AI. Check for mistakes.
|
No description provided.