Skip to content

Fix OidcMetaData MissingImplementation error by removing flawed class path-based test detection#217

Merged
Koosha-Owji merged 4 commits intomainfrom
bugfix/kindeclient-tests-bind-error
Mar 17, 2026
Merged

Fix OidcMetaData MissingImplementation error by removing flawed class path-based test detection#217
Koosha-Owji merged 4 commits intomainfrom
bugfix/kindeclient-tests-bind-error

Conversation

@KomanRudden
Copy link
Copy Markdown
Contributor

@KomanRudden KomanRudden commented Mar 16, 2026

Checking for JUnit in the classpath was was always true causing the implementation error. JUnit check was removed as this was simply not a correct check.

Checklist

🛟 If you need help, consider asking for advice over in the Kinde community.

Summary by CodeRabbit

  • Chores
    • Simplified internal environment configuration logic.
    • Removed legacy test-detection checks.
    • Standardized explicit test-environment initialization in tests.
    • Consolidated test cleanup into guarded finally blocks to improve test reliability.

@KomanRudden KomanRudden requested a review from a team as a code owner March 16, 2026 03:36
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 16, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 1fcd64af-ea60-4dbc-b50d-ed087a2caade

📥 Commits

Reviewing files that changed from the base of the PR and between 9317b13 and e90a62f.

📒 Files selected for processing (2)
  • kinde-j2ee/src/test/java/com/kinde/servlet/KindeSingletonTest.java
  • kinde-management/src/test/java/com/kinde/KindeAdminSessionBuilderTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
  • kinde-j2ee/src/test/java/com/kinde/servlet/KindeSingletonTest.java
  • kinde-management/src/test/java/com/kinde/KindeAdminSessionBuilderTest.java

📝 Walkthrough

Walkthrough

Removed test-environment detection from the Guice module so OidcMetaDataImpl binds whenever the environment state is ACTIVE. Tests were updated to explicitly set the environment to TEST and to initialize/cleanup KindeEnvironmentSingleton where needed.

Changes

Cohort / File(s) Summary
Guice module
kinde-core/src/main/java/com/kinde/client/KindeClientGuiceModule.java
Removed isTestEnvironment() helper and its JUnit-detection logic; conditional binding now checks only for State.ACTIVE.
Core tests
kinde-core/src/test/java/com/kinde/session/KindeClientCodeSessionImplTest.java
Switched test initialization to KindeEnvironmentSingleton.State.TEST; added imports and adjusted test setup/fixtures.
J2EE test
kinde-j2ee/src/test/java/com/kinde/servlet/KindeSingletonTest.java
Added explicit KindeEnvironmentSingleton.init(State.TEST) and fin() calls; wrapped setup/teardown in try/finally.
Management tests
kinde-management/src/test/java/com/kinde/KindeAdminSessionBuilderTest.java
Initialize environment to State.TEST before Guice setup and move cleanup into a finally block calling both KindeGuiceSingleton.fin() and KindeEnvironmentSingleton.fin().

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A little hop in binding land,
Active binds now take their stand,
Tests don TEST and tidy the den,
Init, assert, then fin again,
Hooray — a cleaner rabbit plan! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: removing a flawed JUnit classpath-based test detection mechanism from OidcMetaData binding logic to fix a MissingImplementation error.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bugfix/kindeclient-tests-bind-error
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
kinde-management/src/test/java/com/kinde/KindeAdminSessionBuilderTest.java (1)

38-53: ⚠️ Potential issue | 🟠 Major

Protect singleton teardown with finally to avoid test-state leakage.

Lines 51-52 only run on the happy path. If initialization/build/assertion fails earlier, global singleton state can leak into subsequent tests and cause flakiness.

Proposed fix
 public void testApp() {
     // Initialize Guice with test modules
     KindeEnvironmentSingleton.init(KindeEnvironmentSingleton.State.TEST);
-    KindeGuiceSingleton.init(
-            new KindeManagementGuiceTestModule(),
-            new KindeTokenGuiceTestModule());
-    
-    KindeClient kindeClient = KindeClientBuilder.builder().build();
-    KindeAdminSession kindeAdminSession1 = KindeAdminSessionBuilder.builder().build();
-    KindeAdminSession kindeAdminSession2 = KindeAdminSessionBuilder.builder().client(kindeClient).build();
-    assertTrue( kindeAdminSession1 != kindeAdminSession2 );
-    
-    // Clean up
-    KindeGuiceSingleton.fin();
-    KindeEnvironmentSingleton.fin();
+    try {
+        KindeGuiceSingleton.init(
+                new KindeManagementGuiceTestModule(),
+                new KindeTokenGuiceTestModule());
+
+        KindeClient kindeClient = KindeClientBuilder.builder().build();
+        KindeAdminSession kindeAdminSession1 = KindeAdminSessionBuilder.builder().build();
+        KindeAdminSession kindeAdminSession2 = KindeAdminSessionBuilder.builder().client(kindeClient).build();
+        assertTrue( kindeAdminSession1 != kindeAdminSession2 );
+    } finally {
+        try {
+            KindeGuiceSingleton.fin();
+        } finally {
+            KindeEnvironmentSingleton.fin();
+        }
+    }
 }

As per coding guidelines: focus on error handling and test coverage/quality.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@kinde-management/src/test/java/com/kinde/KindeAdminSessionBuilderTest.java`
around lines 38 - 53, Wrap the test setup and assertions in a try-finally so
singletons are always torn down: move KindeEnvironmentSingleton.init and
KindeGuiceSingleton.init (called in testApp) before the try, put the
KindeClientBuilder/KindeAdminSessionBuilder calls and assertions inside the try
block, and call KindeGuiceSingleton.fin() and KindeEnvironmentSingleton.fin() in
the finally block to guarantee cleanup even on failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@kinde-j2ee/src/test/java/com/kinde/servlet/KindeSingletonTest.java`:
- Around line 37-46: The test currently initializes shared singletons
(KindeEnvironmentSingleton.init, KindeGuiceSingleton.init) and calls
KindeSingleton.getInstance but performs teardown (KindeGuiceSingleton.fin,
KindeEnvironmentSingleton.fin) unguarded; wrap the usage in a try/finally so
fin() is always called even if getInstance or assertions throw, or alternatively
move the fin() calls into a tearDown() method to guarantee cleanup; ensure both
KindeGuiceSingleton.fin and KindeEnvironmentSingleton.fin are invoked inside the
finally/tearDown to prevent singleton state leaking into other tests.

---

Outside diff comments:
In `@kinde-management/src/test/java/com/kinde/KindeAdminSessionBuilderTest.java`:
- Around line 38-53: Wrap the test setup and assertions in a try-finally so
singletons are always torn down: move KindeEnvironmentSingleton.init and
KindeGuiceSingleton.init (called in testApp) before the try, put the
KindeClientBuilder/KindeAdminSessionBuilder calls and assertions inside the try
block, and call KindeGuiceSingleton.fin() and KindeEnvironmentSingleton.fin() in
the finally block to guarantee cleanup even on failures.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bfd80fed-913a-4f15-948c-3d086cb035e3

📥 Commits

Reviewing files that changed from the base of the PR and between 71cee09 and 9317b13.

📒 Files selected for processing (2)
  • kinde-j2ee/src/test/java/com/kinde/servlet/KindeSingletonTest.java
  • kinde-management/src/test/java/com/kinde/KindeAdminSessionBuilderTest.java

@Koosha-Owji
Copy link
Copy Markdown
Contributor

@coderabbitai resume

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 16, 2026

✅ Actions performed

Reviews resumed.

@Koosha-Owji Koosha-Owji merged commit fb53ea6 into main Mar 17, 2026
5 checks passed
@Koosha-Owji Koosha-Owji deleted the bugfix/kindeclient-tests-bind-error branch March 17, 2026 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants