8000 Drop Quakrus container integration test runs by michalvavrik · Pull Request #48664 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

Drop Quakrus container integration test runs - #48664

Merged
vmuzikar merged 1 commit into
keycloak:mainfrom
michalvavrik:feature/issues/48663
May 11, 2026
Merged

Drop Quakrus container integration test runs#48664
vmuzikar merged 1 commit into
keycloak:mainfrom
michalvavrik:feature/issues/48663

Conversation

@michalvavrik
@michalvavrik michalvavrik commented May 3, 2026 •
edited
Loading
Copy link
Copy Markdown
Member

@michalvavrik
michalvavrik requested review from a team as code owners May 3, 2026 21:13
@michalvavrik
michalvavrik requested review from shawkins and vmuzikar May 3, 2026 22:10
@michalvavrik michalvavrik mentioned this pull request May 4, 2026
Closed
@vmuzikar
vmuzikar commented May 6, 2026
Copy link
Copy Markdown
Contributor

Container suite kept intact for:

  • HealthDistTest (9 of 11 methods) — health probes via HTTP on ports 8080/9000
  • MetricsDistTest (8 methods) — metrics via HTTP on port 9000
  • ManagementDistTest (9 methods) — management interface on ports 9000/9005/8080
  • ManagementOffDistTest (1 method) — port 9000 NOT exposed when disabled

I'm wondering whether we should even bring this further and remove those from the container testsuite as well. Is there any container-specific handling there? CC @shawkins

@michalvavrik
michalvavrik commented May 6, 2026 •
edited
Loading
Copy link
Copy Markdown
Member Author

Container suite kept intact for:

  • HealthDistTest (9 of 11 methods) — health probes via HTTP on ports 8080/9000
  • MetricsDistTest (8 methods) — metrics via HTTP on port 9000
  • ManagementDistTest (9 methods) — management interface on ports 9000/9005/8080
  • ManagementOffDistTest (1 method) — port 9000 NOT exposed when disabled

I'm wondering whether we should even bring this further and remove those from the container testsuite as well. Is there any container-specific handling there? CC @shawkins

It is about having a smoke tests for container, more specifically:

  • asserting things that can be asserted by almost any container test like asserting that we use 0.0.0.0 instead of localhost
  • prefer HTTP requests and different ports as tests without these are most likely covered by any tests (like logging)
  • I don't think I found any suitable tests dealing with filesystem (like storing using file log handler) or HTTPS (there is one that disables HTTP and expect failures in the list)

We don't need to keep these specific tests, we just need to run some tests. If you have other tests that should be tested or other criteria I should had applied, please name them. I don't mind to change the selection.

@vmuzikar
vmuzikar commented May 7, 2026
Copy link
Copy Markdown
Contributor

@michalvavrik My thinking here is that maybe we don't need to test container in the Quarkus testsuite at all. A smoke test is done in the Operator testsuite where the container is built too. Maybe that would be enough.

@michalvavrik
michalvavrik commented May 7, 2026
Copy link
Copy Markdown
Member Author

@michalvavrik My thinking here is that maybe we don't need to test container in the Quarkus testsuite at all. A smoke test is done in the Operator testsuite where the container is built too. Maybe that would be enough.

I suppose we don't really care about the difference in the container runtime and new test framework runs clustered tests in docker, so we have some test coverage. I'll drop it entirely then.

@vmuzikar
vmuzikar commented May 7, 2026
Copy link
Copy Markdown
Contributor

I'll drop it entirely then.

+1

@shawkins Are you fine too with removing the container tests completely from the Quarkus testsuite?

@shawkins
shawkins commented May 7, 2026
Copy link
Copy Markdown
Contributor

@shawkins Are you fine too with removing the container tests completely from the Quarkus testsuite?

I'm good with that. We can always add coverage back later if it seems like we're missing something.

@michalvavrik
michalvavrik force-pushed the feature/issues/48663 branch from 0f78d4f to 1dd42c6 Compare May 7, 2026 17:53
@michalvavrik michalvavrik changed the title Reduce Quakrus container integration tests to cover only container-specific aspects while improving test execution time Drop Quakrus container integration test runs May 7, 2026
keycloak-github-bot[bot]
keycloak-github-bot Bot reviewed May 7, 2026
View reviewed changes
@keycloak-github-bot keycloak-github-bot Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

@keycloak-github-bot
keycloak-github-bot Bot commented May 7, 2026
Copy link
Copy Markdown

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.forms.BruteForceTest#testNoFailureResetForPermanentLockout

Keycloak CI - Base IT (5)

org.opentest4j.AssertionFailedError: Expected error event ==> expected: <true> but was: <false>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertTrue.failNotTrue(AssertTrue.java:63)
	at org.junit.jupiter.api.AssertTrue.assertTrue(AssertTrue.java:36)
...

Report flaky test

@michalvavrik
michalvavrik commented May 7, 2026
Copy link
Copy Markdown
Member Author

I've checked the failures but they are not related (Win seems flaky and Infinispan is over Docker pull).

shawkins
shawkins approved these changes May 7, 2026
View reviewed changes
@shawkins shawkins left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

vmuzikar
vmuzikar approved these changes May 11, 2026
View reviewed changes
@vmuzikar vmuzikar left a comment
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks.

The logic for executing the container tests still remains in the testsuite but that is fine – let's keep it for now in case we need to reintroduce the container test runs.

@vmuzikar
vmuzikar force-pushed the feature/issues/48663 branch from 1dd42c6 to d5b7888 Compare May 11, 2026 09:53
@vmuzikar
vmuzikar enabled auto-merge (squash) May 11, 2026 09:53
@michalvavrik
michalvavrik commented May 11, 2026
Copy link
Copy Markdown
Member Author

I think it is clear enough Keycloak CI / Quarkus UT (windows-latest) (pull_request)Failing after 35s is not related, but it prevents auto-merge.

@michalvavrik @vmuzikar
chore(gh ci): drop Quarkus IT container runs
7f751f5 
* Closes: keycloak#48663

Signed-off-by: Michal Vavřík <michal.vavrik@aol.com>
@vmuzikar
vmuzikar force-pushed the feature/issues/48663 branch from d5b7888 to 7f751f5 Compare May 11, 2026 14:17
@vmuzikar
vmuzikar merged commit 295b401 into keycloak:main May 11, 2026
89 checks passed
@michalvavrik
michalvavrik deleted the feature/issues/48663 branch May 11, 2026 15:42
@stianst stianst mentioned this pull request Jun 1, 2026
Closed
This was referenced Jun 8, 2026
Closed
Merged
stianst pushed a commit that referenced this pull request Jun 8, 2026
@michalvavrik
chore(gh ci): drop Quarkus IT container runs (#48664) (#49783)
d38589d 
* Closes: #48663


(cherry picked from commit 295b401)

Signed-off-by: Michal Vavřík <michal.vavrik@aol.com>
sashyo pushed a commit to tide-foundation/keycloak-IGA that referenced this pull request Jul 20, 2026
@michalvavrik @sashyo
chore(gh ci): drop Quarkus IT container runs (keycloak#48664)
049b175 
* Closes: keycloak#48663

Signed-off-by: Michal Vavřík <michal.vavrik@aol.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shawkins shawkins shawkins approved these changes
@vmuzikar vmuzikar vmuzikar approved these changes
+1 more reviewer
@keycloak-github-bot keycloak-github-bot[bot] keycloak-github-bot[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

flaky-test team/cloud-native

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Quarkus integration tests should only run in container when they verify container-specific behavior

3 participants

@michalvavrik @vmuzikar @shawkins
0