UNOMI-139 UNOMI-878: Platform integration tests#766
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds new PaxExam-based integration tests to cover multitenancy, security/system execution context behavior, scheduler REST API flows, persona sessions, and event collector authentication; also updates the AllITs suite ordering to include the new tests and run HealthCheckIT last to reduce flakiness.
Changes:
- Added new integration test classes:
TenantIT,SecurityIT,SchedulerIT,PersonaIT,EventsCollectorIT. - Added persona test payload resource for creating a persona with sessions via REST.
- Updated
AllITsto include the new ITs and moveHealthCheckITto the end.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| itests/src/test/resources/persona/persona-with-sessions-payload.json | Adds payload used by persona/session integration test. |
| itests/src/test/java/org/apache/unomi/itests/TenantIT.java | Adds multitenancy and API key/auth integration coverage. |
| itests/src/test/java/org/apache/unomi/itests/SecurityIT.java | Adds system subject/context execution and restoration coverage. |
| itests/src/test/java/org/apache/unomi/itests/SchedulerIT.java | Adds scheduler task REST API integration coverage. |
| itests/src/test/java/org/apache/unomi/itests/PersonaIT.java | Adds persona-with-sessions creation and retrieval coverage. |
| itests/src/test/java/org/apache/unomi/itests/EventsCollectorIT.java | Adds event collector authentication/flags coverage. |
| itests/src/test/java/org/apache/unomi/itests/AllITs.java | Registers new ITs and moves HealthCheckIT to run last. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+388
to
+394
| Tenant tenant = tenantService.createTenant("expired-tenant", Collections.emptyMap()); | ||
| ApiKey apiKey = tenantService.generateApiKey(tenant.getItemId(), 1L); // 1ms validity | ||
|
|
||
| Thread.sleep(2); // Wait for key to expire | ||
|
|
||
| Assert.assertFalse(tenantService.validateApiKey(tenant.getItemId(), apiKey.getItemId())); | ||
| } |
Contributor
Author
There was a problem hiding this comment.
Fixed. The call was corrected to apiKey.getKey(), which passes the actual secret value as expected by validateApiKey(). The test now also cleans up the tenant in a finally block.
7cb94e6 to
8fef941
Compare
c06d88c to
81efb61
Compare
8fef941 to
70b8c49
Compare
81efb61 to
6bdc8c9
Compare
70b8c49 to
7897291
Compare
6bdc8c9 to
95c656c
Compare
7897291 to
03481dd
Compare
95c656c to
bc248ea
Compare
03481dd to
d9512ab
Compare
bc248ea to
4f98f34
Compare
d9512ab to
b5003e0
Compare
4f98f34 to
de19c58
Compare
Add TenantIT, SecurityIT, PersonaIT, EventsCollectorIT, SchedulerIT and persona test fixture. Register platform ITs in AllITs (including TenantIT, SchedulerIT, EventsCollectorIT for CI coverage per split plan).
- TenantIT: use /cxs/context.json (REST endpoint) instead of legacy /context.json servlet forward that timed out under Pax Exam. - EventCollectorResponse: add Jackson @JsonCreator so IT clients can deserialize the POST /cxs/eventcollector JSON body.
…lures" This reverts commit 958d2fb.
Avoid heavy concurrent /health/check load immediately before TenantIT.
de19c58 to
96e9396
Compare
Fix several bugs found during review: - TenantIT: two JAAS auth tests were reusing request objects that still had the previous auth header set, so they were not actually testing JAAS — now each JAAS assertion uses a fresh request - TenantIT: apiKey.getItemId() was passed instead of apiKey.getKey() to validateApiKey(), making the expiry test always return false - SchedulerIT: testRetryTask was waiting for the task executor to run and fail organically, but the test node may not be the scheduler executor node — now uses saveTask() to put the task in FAILED state directly before calling the retry endpoint Improve test reliability: - All tests that create tenants now clean up in a finally block, so a test failure does not leave state behind for the next test - HTTP responses are now closed with try-with-resources in all tests - Thread.sleep(1500) replaced with keepTrying() polling Clean up dead code: - Remove unused constants and imports from EventsCollectorIT, SecurityIT, and SchedulerIT - PersonaIT setUp() was returning a closed HTTP response through the keepTrying chain — now returns Boolean.TRUE instead Add no-arg constructor to EventCollectorResponse for Jackson deserialization compatibility
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.
Summary
TenantIT,SecurityIT,PersonaIT,SchedulerIT, andEventsCollectorITintegration test classes covering multitenancy and core platform flowsTenantITandEventsCollectorITtest failures (reverted intermediate fix, replaced with stable version)HealthCheckITto run last inAllITssuite to avoid startup-timing flakinessTest plan
./build.sh --ci --integration-testsrunAllITssuite ordering change does not affect other testsUNOMI-880-878-shell-dev-commands— merge that first🤖 Generated with Claude Code