Repository navigation
Refactor and unify RunOnServer modules - #48363
Conversation
There was a problem hiding this comment.
This can be simplified and made cleaner.
For example if you have RunOnServerUtils has something like:
public static RunOnServer printRealm(String realm) {
return session -> System.out.println(session.realms().getRealmByName(realm).getDisplayName());
}
Then you can use it with runOnServer.run(RunOnServerUtils.printRealm("asdfsadf")), which removes the need to have the session -> part and also passing session to the method.
You should probably also call it RunOnServerHelpers instead of Utils as that's what we have used for those things in the past.
Finally, this shouldn't be part of the test-framework as these are very internal things, that we don't want to expose. So just add it to the tests/base.
|
@vaceksimon should be in |
|
Well, right now I have two versions of Besides, I don't think we get more benefit from moving it to |
|
@stianst or maybe this new helper should be merged with the already existing |
57250cd to
b3b258a
Compare
There was a problem hiding this comment.
Hi Simon, thank you for this PR @vaceksimon.
I think using RunHelpers is fine, also moving it to the utils-shared folder for a temporary time. But splitting the remote-providers doesn't make sense, it creates a dependency mess.
I think, you should adjust old test suite into a new one not vise versa (old TimeOffset provider has different package name). Please revert those module changes.
Also move the LoginTest migration into a separate PR.
Wdyt?
cc @stianst
| <dependency> | ||
| <groupId>org.keycloak.testframework</groupId> | ||
| <artifactId>keycloak-test-framework-remote-runonserver</artifactId> | ||
| <version>999.0.0-SNAPSHOT</version> |
There was a problem hiding this comment.
shouldn't it be
| <version>999.0.0-SNAPSHOT</version> | |
| <version>${keycloak.version}</version> |
There was a problem hiding this comment.
good catch, thanks
|
Thanks @lhanusov
Yeah, spliting the module doesn't provide any real benefits, I'll revert that
Yep, I rewrote only one test method to see the run on server works in both test suites. |
407e0df to
19e2af1
Compare
Signed-off-by: Simon Vacek <simonvacky@email.cz>
19e2af1 to
dae0b82
Compare
Signed-off-by: Simon Vacek <simonvacky@email.cz>
Closes #48707
To make migrating tests easier for RunOnServer added
TestingResourcemethods toRunHelpers. To have only one copy, I removed legacy test suite's runonserver code and added a dependency to the new test framework.What changed:
RunOnServer,FetchOnServer,SerializationUtil, etc. from testsuite/integration-arquillian/testsuite-providersdepend onkeycloak-test-framework-remote-providersOther cleanup: