+
Skip to content

Testsuite of keycloak-admin-client in keycloak-client repository #16

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 5, 2024
Merged

Testsuite of keycloak-admin-client in keycloak-client repository #16

merged 2 commits into from
Aug 5, 2024

Conversation

mposolda
Copy link
Contributor

@mposolda mposolda commented Aug 1, 2024 •
edited
Loading

closes keycloak/keycloak#31867

Adding 1st version of the testsuite for the keycloak-admin-client and keycloak-admin-client-jee in the keycloak-client repository.

  • Testsuite uses white-box approach. The aim is to not have any dependencies on keycloak to avoid clash of the classes of keycloak-client modules with the Keycloak classes.

  • Keycloak server start/stop is based on testcontainers and by default, which is the approach we discussed with Stian as possible way to go. It is running the Keycloak server inside docker image. It introduces dependency on keycloak-testcontainers, which is 3rd party library, but should be OK since it is just for testing purposes. Was also thinking about forking that (just around 3 classes), but added depenency for now.

  • So far, the server is executed separately for each test class, but this may be improved as a follow-up to possibly run the server for the whole testsuite

  • Ability to run the testsuite against Keycloak server, which is executed locally (In this case, testsuite won't start/stop Keycloak server). Can be faster turnaround or possibility to easily debug Keycloak server if needed

  • The hope is to not duplicate test classes for keycloak-admin-client and keycloak-admin-client-jee and re-use them as much as possible.

Possible follow-up tasks:

  • Ability to test with different versions of Keycloak server. Test with last 2 versions of Keycloak server (so 24 and 25) + Keycloak nightly in GH actions

  • Add more tests (or investigate if we can re-use the tests from the package org.keycloak.testsuite.admin from Keycloak testsuite, but does not seem feasible to me ATM)

@mposolda mposolda self-assigned this Aug 1, 2024
@mposolda mposolda force-pushed the 30804-testsuite-rebase branch from 29f3e98 to 40129bd Compare August 2, 2024 12:19
@mposolda mposolda mentioned this pull request Aug 2, 2024
Closed
@mposolda mposolda force-pushed the 30804-testsuite-rebase branch from 40129bd to e42f69e Compare August 2, 2024 14:08
@rmartinc
Copy link
Contributor

rmartinc commented Aug 5, 2024

@mposolda IMHO using test-containers is very good idea. So I like the approach in this PR.

But I really hate to repeat the tests for admin-client-jee-tests and admin-client-tests as it is now (extending the class). We need to think something to just create the test in one project and somehow execute it for both implementations. Maybe we can just create one project with two profiles: javaee and jakartaee (default). But probably we will need direct jax-rs imports and then we will need two projects and transformation.

@mposolda
Update per review - Generate tests in admin-client-tests from admin-c…
0c5d954 
…lient-jee-tests

Signed-off-by: mposolda <mposolda@gmail.com>
@mposolda
Copy link
Contributor Author

mposolda commented Aug 5, 2024

@rmartinc Thanks for the feedback! I've updated this PR (used separate commit just for the easier review, so you can easily see the changes).

I've updated this to:

  • Still have 2 separate modules in the testsuite admin-client-tests and admin-client-jee-tests to make sure that classpath is correctly isolated and the JEE version is really testing with JEE admin-client and correct versions of RestEasy etc. But updated PR to use the "jakarta transformer plugin" to generate the tests in the admin-client-tests from the admin-client-jee-tests. Agree that it is probably much better than using inheritance and allows that admin-client-tests and admin-client-jee-tests don't have any dependencies on each other.

I've kept only ClasspathJEETest and ClasspathJakartaTest, which just uses some reflection to test that corresponding module is really testing with the correct version of admin client (JEE with keycloak-admin-client-jee and Jakarta with keycloak-admin-client). But I assume that this may be the only test repeated in the modules. Most of the tests would be in the package org.keycloak.client.testsuite.adminclient and hence will be present just on the single place.

WDYT?

rmartinc
rmartinc approved these changes Aug 5, 2024
View reviewed changes
Copy link
Contributor

@rmartinc rmartinc left a comment

Choose a reason for hiding this comment

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

LGTM @mposolda! Probably we'll need more tuning as long as we create more tests classes, but it's a very nice starting point.

@mposolda
Copy link
Contributor Author

mposolda commented Aug 5, 2024

@rmartinc Thanks! I am merging (I already have some WiP on the follow-up for the ability to test nightly builds with more Keycloak versions). Agree that we will need more tuning and improvements (like EG don't require to start/stop the server for every test etc).

@mposolda mposolda merged commit cec8cb3 into keycloak:main Aug 5, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rmartinc rmartinc rmartinc approved these changes

Assignees

@mposolda mposolda

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Adding initial testsuite
2 participants
@mposolda @rmartinc
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载