ci: richer integration-test reporting + AIO sector_identifier whitelist - #14679
Merged
Conversation
Reporting (summarize_testng.py): - group results into a per-backend table of contents with per-module (total / failed) counts instead of a flat class list - list every failing test with its TestNG data-provider input parameters, the way Jenkins surfaced "Parameter #1/#2/#3", so a failure can be triaged from the summary without digging through the raw logs - add a --combined mode and an aggregate job that stitches every matrix leg into one global view (MySQL + PostgreSQL side by side) sector_identifier_uri: - populate externalUriWhiteList with the running host so loop-back sector_identifier_uri / request_uri fetches to the AIO (which resolves to an RFC1918 docker IP) aren't rejected as private addresses; the persistence-loader delta previously left it empty and the linux-setup loader still pinned the retired jenkins-build.jans.io host Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com>
mo-auto
requested review from
devrimyatar,
iromli,
moabu,
yurem,
yuriyz and
yuriyzz
as code owners
August 3, 2026 10:34
Member
Author
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
mo-auto
had a problem deploying
to
integration-tests
August 3, 2026 10:35 — with 
GitHub Actions
Failure
GitHub Actions
Failure
mo-auto
had a problem deploying
to
integration-tests
August 3, 2026 10:35 — with
GitHub Actions
Failure
GitHub Actions
Failure
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe TestNG summarizer now supports module-aware per-leg and combined reports. The integration workflow aggregates matrix reports, cleans stale droplets, and improves provisioning diagnostics. Test-data loaders use configured hostnames in external URI whitelists. Dockerfiles add retry handling to Prometheus JMX exporter downloads. ChangesTestNG reporting
Integration provisioning
Runtime hostname configuration
Prometheus JMX downloads
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
mo-auto
added
area-CI
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/scripts/summarize_testng.py:
- Around line 175-179: Update the per-leg reporting branch around _failing_rows
to emit “_No results collected._” when stats["total"] == 0, while preserving the
distinct-failures message for non-empty all-pass results. Apply the same
no-results handling to the combined report at
.github/workflows/scripts/summarize_testng.py lines 201-204: emit “_No results
collected._” when no combined leg has records, and explicitly identify each
zero-record backend in mixed runs.
- Around line 79-84: Update the record key construction in the TestNG
summarization flow to preserve parameter boundaries unambiguously, rather than
joining raw values with "|". Ensure empty parameter lists remain distinct from
lists containing empty values and distinct value sequences cannot collide, while
leaving the existing status ranking and record replacement logic unchanged.
- Around line 110-120: Update _md_cell to HTML-escape input text before applying
its existing backslash, pipe, newline, and trimming transformations, ensuring
values such as <br> and closing tags render literally while preserving the
current Markdown cell behavior.
In `@jans-linux-setup/jans_setup/setup_app/test_data_loader.py`:
- Line 261: Update the test-data setup around set_jans_auth_conf_dynamic so
externalUriWhiteList is read from the existing configuration, merged with
Config.hostname, deduplicated, sorted, and then persisted. Replace the direct
single-host list assignment while preserving all existing whitelist entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 194525b2-c7be-4cdd-983a-8a4cd5cb3c8a
📒 Files selected for processing (4)
.github/workflows/scripts/summarize_testng.py.github/workflows/test-integration.ymldocker-jans-persistence-loader/scripts/test_data_setup.pyjans-linux-setup/jans_setup/setup_app/test_data_loader.py
yuriyz
requested changes
Aug 3, 2026
| 'sessionIdRequestParameterEnabled': True, | ||
| 'skipRefreshTokenDuringRefreshing': False, | ||
| 'featureFlags': ['unknown', 'health_check', 'userinfo', 'clientinfo', 'id_generation', 'registration', 'introspection', 'revoke_token', 'global_token_revocation', 'end_session', 'status_session', 'jans_configuration', 'ciba', 'uma', 'u2f', 'device_authz', 'stat', 'par', 'ssa', 'status_list', 'logout_status_jwt', 'access_evaluation', 'identity_assertion_authz_grant'], | ||
| 'externalUriWhiteList':['jenkins-build.jans.io'], |
Contributor
There was a problem hiding this comment.
@moabu lets keep jenkins-build.jans.io until we fully move github. I need the way to track all integration tests.
Member
There was a problem hiding this comment.
I replaced it with Config.hostname so it can be passed. Do you want it to be statically defined ?
`wget` treats an HTTP 500 as fatal, so a transient GitHub-releases 500 while fetching jmx_prometheus_javaagent aborts the whole image build (seen on the config-api image in an integration-test run). Add GNU wget retry flags (--tries/--retry-connrefused/--retry-on-http-error/--waitretry/-T) so server errors and connection drops back off and retry rather than kill the build. Applied to every service image that downloads the agent. Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com>
mo-auto
had a problem deploying
to
integration-tests
August 3, 2026 11:35 — with
GitHub Actions
Failure
GitHub Actions
Failure
mo-auto
had a problem deploying
to
integration-tests
August 3, 2026 11:35 — with
GitHub Actions
Failure
GitHub Actions
Failure
coderabbitai
Bot
added
comp-docker-jans-auth-server
comp-docker-jans-config-api
comp-docker-jans-fido2
comp-docker-jans-scim
comp-docker-jans-link
comp-docker-jans-casa
The provision step used `curl -f`, which discards the response body and reports only "exit code 22" when the API returns a non-2xx. A 422 (droplet/SSH-key limit reached, invalid size/region, ...) was therefore undiagnosable. Capture the HTTP status and echo the API's own error message, and log the current droplet count on the account (a full account is the usual cause of a create 422, and a reap/teardown leak would show up there). Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com>
mo-auto
had a problem deploying
to
integration-tests
August 3, 2026 11:45 — with
GitHub Actions
Failure
GitHub Actions
Failure
Contributor
✅ Action performedComments resolved and changes approved. |
The combined summary listed every failing test inline, making the page huge on a large suite. Keep the matrix always visible, and move the detail into two default-collapsed groups: "Failed tests" (with input parameters) and "Passed tests" (class/method only, to stay small). Passed is emitted last so a step- summary size cap truncates there, leaving the matrix and failures intact. Matrix failed counts link to the Failed group heading. Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com>
mo-auto
temporarily deployed
to
integration-tests
August 6, 2026 11:24 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
mo-auto
temporarily deployed
to
integration-tests
August 6, 2026 11:24 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Apply the same default-collapsed Failed / Passed groups to each leg's (MYSQL/PGSQL) step summary that the combined view uses, so those pages stay short as well. Drops the now-unused open failing-table helper. Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com>
mo-auto
had a problem deploying
to
integration-tests
August 6, 2026 13:09 — with
GitHub Actions
Failure
GitHub Actions
Failure
mo-auto
had a problem deploying
to
integration-tests
August 6, 2026 13:09 — with
GitHub Actions
Failure
GitHub Actions
Failure
Render 'failed: N' on every module row and 'total / N' in every matrix cell (not only when N>0), so the failed count is visible for all modules rather than appearing to exist only for the ones that happen to have failures. Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com>
Use PR-built assets end to end (so a fix under test is actually exercised): - parameterise the auth-server/scim/fido2 Dockerfiles with CN_RELEASE_DOWNLOAD_URL (mirroring config-api), default unchanged - run_aio_integration.sh now builds local auth-server/scim/fido2 images from the :8088 PR artifacts and passes them as AIO build args, so the running server reflects this checkout -- not just the client-side suites Module selection (default all): - workflow_dispatch exposes an "all modules" checkbox plus one per module; a resolve step turns the ticks into TEST_MODULES - run_aio_integration.sh runs only the selected suites; the reactor and AIO are always built in full so build dependencies are honoured Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com>
summarize_testng.py read only testng-results.xml, so a test that emits only a JUnit TEST-*.xml (e.g. io.jans.as.server.comp.db.UserJansExtUidAttributeTest) was invisible and its failures uncounted. Fold JUnit testcases into the tally for (class, method) pairs TestNG never reported, without double-counting the JUnit copies surefire also writes for TestNG tests. Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com>
Each leg's step summary and the combined summary now end with "Jump to: MySQL · PostgreSQL · Combined summary" #summary-<job-id> permalinks, and the Zulip message carries the same section links, so a reader lands on the right section immediately instead of scrolling the run. Job ids come from the run's jobs API (needs actions: read). Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com>
mo-auto
had a problem deploying
to
integration-tests
August 7, 2026 11:52 — with
GitHub Actions
Failure
GitHub Actions
Failure
mo-auto
had a problem deploying
to
integration-tests
August 7, 2026 11:52 — with
GitHub Actions
Failure
GitHub Actions
Failure
Member
|
@coderabbitai approve |
moabu
had a problem deploying
to
integration-tests
August 11, 2026 16:15 — with
GitHub Actions
Failure
GitHub Actions
Failure
moabu
had a problem deploying
to
integration-tests
August 11, 2026 16:15 — with
GitHub Actions
Failure
GitHub Actions
Failure
Contributor
✅ Action performedComments resolved and changes approved. |
1 similar comment
Contributor
✅ Action performedComments resolved and changes approved. |
moabu
approved these changes
Aug 11, 2026
|
Dahkenangnon
pushed a commit
that referenced
this pull request
Aug 12, 2026
…st (#14679) * ci: richer integration-test reporting + AIO sector_identifier whitelist Reporting (summarize_testng.py): - group results into a per-backend table of contents with per-module (total / failed) counts instead of a flat class list - list every failing test with its TestNG data-provider input parameters, the way Jenkins surfaced "Parameter #1/#2/#3", so a failure can be triaged from the summary without digging through the raw logs - add a --combined mode and an aggregate job that stitches every matrix leg into one global view (MySQL + PostgreSQL side by side) sector_identifier_uri: - populate externalUriWhiteList with the running host so loop-back sector_identifier_uri / request_uri fetches to the AIO (which resolves to an RFC1918 docker IP) aren't rejected as private addresses; the persistence-loader delta previously left it empty and the linux-setup loader still pinned the retired jenkins-build.jans.io host Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> * ci(docker): retry the jmx_exporter download instead of failing the build `wget` treats an HTTP 500 as fatal, so a transient GitHub-releases 500 while fetching jmx_prometheus_javaagent aborts the whole image build (seen on the config-api image in an integration-test run). Add GNU wget retry flags (--tries/--retry-connrefused/--retry-on-http-error/--waitretry/-T) so server errors and connection drops back off and retry rather than kill the build. Applied to every service image that downloads the agent. Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> * ci: surface the DigitalOcean API error body on droplet provisioning The provision step used `curl -f`, which discards the response body and reports only "exit code 22" when the API returns a non-2xx. A 422 (droplet/SSH-key limit reached, invalid size/region, ...) was therefore undiagnosable. Capture the HTTP status and echo the API's own error message, and log the current droplet count on the account (a full account is the usual cause of a create 422, and a reap/teardown leak would show up there). Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> * ci: fall back across regions when a droplet size is unavailable DigitalOcean returns HTTP 422 "Size is not available in this region" when the pinned size (s-8vcpu-16gb) has no capacity in nyc1, which failed the whole run on a transient regional shortage. Try the size across several regions and take the first that accepts, so provisioning survives per-region availability drift. Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> * ci: harden TestNG summary (unambiguous keys, escaping, no-results) - key records by a params tuple, not a "|"-join, so [] != [""] and ["a|b"] != ["a","b"] no longer collide into one record - HTML-escape Markdown cell values so a parameter containing <br> or a closing </details> renders literally instead of injecting markup - distinguish "no results collected" (nothing ran) from "no distinct failures" (all-pass) in both the per-leg and combined reports, and name each empty backend in a mixed combined run Also merge externalUriWhiteList in the linux-setup loader with the existing configured entries (read + dedupe + sort) instead of overwriting them. Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> * ci: preflight sweep of leaked CI droplets before provisioning A run that crashes before teardown leaks its droplet (reap only covers same-run resources), and these accumulate toward the account's droplet limit. Delete jans-ci-tagged droplets older than 2h before provisioning; the age gate ensures a concurrent run's freshly-created droplet is never swept. Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> * ci: bound DO API calls and tighten provisioning failure handling - add connect/total timeouts to the DO API POSTs, kept non-retrying so a client timeout can't duplicate an already-accepted create - persist key_id to GITHUB_OUTPUT as soon as the SSH key is registered (and validate the response with jq -er), so teardown can delete the key even when droplet creation fails; droplet_id is written only after its create succeeds - the region loop now advances only on the documented "size not available in this region" 422; quota 422 / 429 / 5xx abort immediately with the API message - the droplet-count probe fails closed to '?' (never a false 0) and uses meta.total so it reflects the whole account Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> * ci: bound the stale-droplet sweep curls with timeouts The sweep's droplet-list GET and per-droplet DELETE had no timeouts, so a hung DO API connection could stall the job before provisioning. Add connect/total timeouts matching the other DO calls; the DELETE stays non-retrying. Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> * ci: compact global matrix + Zulip report for integration tests - lead the combined summary with a backends × modules glance table (distinct / failed per cell) so MySQL and PostgreSQL are comparable at the top without scrolling to each backend's section - add a --zulip message mode and post it to Zulip #bot_reporter (topic "integration-tests") with a link to the run, mirroring the flex admin-ui report; gated to non-PR events and needs the ZULIP_API_KEY secret Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> * ci: speed up the MySQL leg with relaxed durability + timeout headroom The MySQL integration leg ran ~2.4x slower than PostgreSQL (auth-client suite 29m vs 12m, plus a slower AIO startup) and tipped over the 2h cap, losing its results to cancellation. MySQL was running with default per-commit fsync and binary logging, which dominates this write-heavy test-data load. Relax durability on the throwaway demo/CI database (flush-log-at-trx-commit=2, doublewrite off, binlog off) and raise the job timeout to 150m for headroom. Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> * ci: clickable global matrix, Zulip on every run, queue instead of cancel - combined summary now leads with the matrix and drops the redundant per-backend bullets; each non-zero failed count links to that backend's failing-tests table (anchored "### <backend> failures") for a one-click drill-down - post the Zulip report on every run, pass or fail (always()), instead of only non-PR events, with a fallback message if summary generation fails - concurrency cancel-in-progress: false so a newer commit queues behind an in-flight run rather than cancelling it and losing a nearly-finished leg Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> * ci: cancel a ref's superseded run instead of queueing Revert to cancel-in-progress: true. The concurrency group is scoped to github.ref, so it only ever cancels this same branch/PR's older run -- never another developer's -- and a superseded run is testing code a newer push already replaced, so cancelling it discards nothing useful while avoiding a run queue. Losing a nearly-finished run to the 2h cap was a timeout concern, handled separately by the MySQL speedup + timeout-minutes headroom. Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> * ci: trim over-commenting in the integration-test changes Condense the multi-line rationale comments added across this branch to concise WHY-only notes; no behaviour change. Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> * ci: collapse pass/fail test lists under the combined matrix The combined summary listed every failing test inline, making the page huge on a large suite. Keep the matrix always visible, and move the detail into two default-collapsed groups: "Failed tests" (with input parameters) and "Passed tests" (class/method only, to stay small). Passed is emitted last so a step- summary size cap truncates there, leaving the matrix and failures intact. Matrix failed counts link to the Failed group heading. Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> * ci: collapse pass/fail lists in the per-leg summaries too Apply the same default-collapsed Failed / Passed groups to each leg's (MYSQL/PGSQL) step summary that the combined view uses, so those pages stay short as well. Drops the now-unused open failing-table helper. Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> * ci: always show the per-module failed counter Render 'failed: N' on every module row and 'total / N' in every matrix cell (not only when N>0), so the failed count is visible for all modules rather than appearing to exist only for the ones that happen to have failures. Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> * ci: build the AIO from PR artifacts + allow selecting test modules Use PR-built assets end to end (so a fix under test is actually exercised): - parameterise the auth-server/scim/fido2 Dockerfiles with CN_RELEASE_DOWNLOAD_URL (mirroring config-api), default unchanged - run_aio_integration.sh now builds local auth-server/scim/fido2 images from the :8088 PR artifacts and passes them as AIO build args, so the running server reflects this checkout -- not just the client-side suites Module selection (default all): - workflow_dispatch exposes an "all modules" checkbox plus one per module; a resolve step turns the ticks into TEST_MODULES - run_aio_integration.sh runs only the selected suites; the reactor and AIO are always built in full so build dependencies are honoured Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> * ci: count pure-JUnit tests missing from the TestNG reports summarize_testng.py read only testng-results.xml, so a test that emits only a JUnit TEST-*.xml (e.g. io.jans.as.server.comp.db.UserJansExtUidAttributeTest) was invisible and its failures uncounted. Fold JUnit testcases into the tally for (class, method) pairs TestNG never reported, without double-counting the JUnit copies surefire also writes for TestNG tests. Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> * ci: deep-link the per-backend and combined summaries Each leg's step summary and the combined summary now end with "Jump to: MySQL · PostgreSQL · Combined summary" #summary-<job-id> permalinks, and the Zulip message carries the same section links, so a reader lands on the right section immediately instead of scrolling the run. Job ids come from the run's jobs API (needs actions: read). Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> --------- Signed-off-by: moauto <54212639+mo-auto@users.noreply.github.com> Co-authored-by: Mohammad Abudayyeh <47318409+moabu@users.noreply.github.com>
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
3 participants
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.
What
Three improvements to the integration-test (
test-integration.yml) reporting and setup, driven by pain points investigating AIO failures.1. Per-backend table of contents with per-module counts
summarize_testng.pynow groups results into a per-backend ToC with per-module(total / failed)counts instead of a flat failing-class list:2. Input parameters on every failing test
Each failing test is listed with its TestNG data-provider values, numbered the way Jenkins surfaced "Parameter #1/#2/#3", so a failure can be triaged from the summary without digging through the raw logs:
ParHttpTestregisterPar2: …/sectoridentifier/a55ede29
3. Global view across backends
A new
--combinedmode and anaggregatejob stitch every matrix leg into one step summary (MySQL + PostgreSQL side by side), with a combined failing-tests table that adds a Backend column. The per-leg summaries remain.4.
externalUriWhiteListpopulated with the AIO hostRoot cause of the
invalid_client_metadata/ sector_identifier rejections: the AIO resolves to an RFC1918 docker IP, andSectorIdentifierUriServicefails closed on private addresses unless the host is explicitly whitelisted. The persistence-loader test-data delta leftexternalUriWhiteListempty, and the linux-setup loader still pinned the retiredjenkins-build.jans.io. Both now use the running host.Notes
dorny/test-reporterPR check (that reporter reads JUnitTEST-*.xml, which doesn't carry structured data-provider params).Closes fix: ci: richer integration-test reporting + AIO sector_identifier whitelist -autocreated #14680,
Summary by CodeRabbit
New Features
Bug Fixes
Reliability Improvements