mirror/keycloak
1
0
Fork 0
mirror of https://github.com/keycloak/keycloak.git synced 2025-12-16 12:05:49 -06:00
Code Issues Packages Projects Releases Wiki Activity

Migration util imrovements (#39161)

Browse Source 
Closes #37691
Implements #37283



migration util rewrite static imports



migration util add more assert methods



migration util rewrite common statements



migration util fix diff tool usage



Update test migration README



migration util review fixes

Signed-off-by: Simon Vacek <simonvacky@email.cz>
This commit is contained in:
Šimon Vacek
2025-05-19 09:55:25 +02:00
committed by GitHub
parent 5e940c011a
commit 175102d03d
8 changed files with 252 additions and 101 deletions
Download Patch File Download Diff File Expand all files Collapse all files

77
tests/MANUAL_MIGRATION.md Normal file
View File

@@ -0,0 +1,77 @@
### Basics
Check the [MIGRATING_TESTS guide](./MIGRATING_TESTS.md) first if you have not already done so.
When migrating tests use the remote server mode as this will make it much quicker to run tests than having to start/stop
Keycloak when you run the test from the IDE.
Add `@KeycloakIntegrationTest` to the class.
Change `import org.junit.Test;` to `import org.junit.jupiter.api.Test;`
Change `org.junit.Before` to `org.junit.jupiter.api.BeforeEach`
Remove extends `AbstractKeycloakTest` as the new test framework provides injection of resources needed by the test there
is no need for the `AbstractKeycloakTest` and tests should instead inject what they need.
One thing your test is most likely going to need is a realm, this is now done with:
```
@InjectRealm
ManagedRealm realm;
```
With this change, most of the time you do not need to get the `RealmResource` via an admin client. Instead, you can use
`realm.admin()`
### Changed packages/classes
| Old | New |
|---------------------------------------------|------------------------------------------------|
| org.junit.Assert | org.junit.jupiter.api.Assertions |
| org.keycloak.testsuite.Assert | org.keycloak.tests.utils.Assert |
| org.junit.Test | org.junit.jupiter.api.Test |
| org.keycloak.testsuite.admin.ApiUtil | org.keycloak.tests.utils.admin.ApiUtil |
| org.keycloak.testsuite.util.AdminEventPaths | org.keycloak.tests.utils.admin.AdminEventPaths |
### Assertions
Change `import org.junit.Assert;` to `import org.junit.jupiter.api.Assertions;`, and replace `Assert.` with `Assertions.` throughout.
If the assert passes a message (for example `Assert.assertEquals("Message", expected, actual)`) the message in `Assertions`
is now the last parameter (for example `Assertions.assertEquals(expected, actual, "Message")`).
### Admin events
Admin events are handled slightly differently in the new test framework.
An example for the old testsuite:
```
@Rule
public AssertAdminEvents assertAdminEvents = new AssertAdminEvents(this);
public void myTest() {
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, ...);
}
```
Converted to the new test framework:
```
@InjectAdminEvents
public AdminEvents adminEvents;
public void myTest() {
AdminEventAssertion.assertEvent(adminEvents.poll(), OperationType.CREATE, ...);
}
```
Notice that there is no need to pass `realmId` when asserting an event, that is because the `AdminEvents` will only
receive events for the current realm.
For better readability `AdminEventAssertion` provides a method chaining approach to assert various fields in the event
(the example above could be change to `AdminEventAssertion.assertSuccess(adminEvents.poll()).operationType(OperationType.CREATE)...`).
There is also improved support for skipping events using skip methods, that allows skipping one event (`.skip()`),
multiple events (`.skip(5)`), or skipping all previous events (`.skipAll()`).

129
tests/MIGRATING_TESTS.md
View File

@@ -1,69 +1,78 @@
### Basics
# How to migrate tests to the new framework?
When migrating tests use the remote server mode as this will make it much quicker to run tests than having to start/stop
## TLDR
1. cd into the [`migration-util` module](./migration-util)
2. Use the [migration script](./migration-util/migrate.sh)
```shell
./migrate.sh SomeTest
```
**The script doesn't work?** Follow the [MANUAL_MIGRATION guide](./MANUAL_MIGRATION.md).
3. Fix the rest of the test class. To speed up the process, use the remote server mode when running the test.
4. Use the [commit-migration script](./migration-util/commit-migration.sh) to commit the changes correctly:
```shell
./commit-migration.sh
```
5. On the PR on GitHub, review the commit that modifies the files
6. Do not squash the commits when merging the PR
## Migration process
Migrating tests involves doing a lot of repetitive tasks. We made some automation tooling in the
[`migration-util` module](./migration-util) to make it less annoying.
### Migrating test classes
To migrate a test class, you can use the [migrate script](./migration-util/migrate.sh).
cd into the [`migration-util` module](./migration-util) and run:
```shell
./migrate.sh SomeTest
```
The script accepts one parameter that can be either a class name or an absolute path to the file. By default, look-up
starts from [`testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite`](../testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite).
When run, the script copies the test class and rewrites or adds common test statements, such as:
- adds the `@KeycloakIntegrationTest` annotation
- changes JUnit 4's `@Before` to JUnit 5's `@BeforeEach`
- updates JUnit 4's assertions with their JUnit 5's counterparts
And more.
Besides the printed logs, you can pass the script a diff tool to see the changes made:
```shell
DIFFTOOL="diff --color=always" ./migrate.sh SomeTest
```
```shell
DIFFTOOL="meld" ./migrate.sh SomeTest
```
If the script fails and throws an exception, you can try to fix it or refer to the
[MANUAL_MIGRATION guide](./MANUAL_MIGRATION.md).
The migrated test shall be in the same package in
[`tests/base/src/test/java/org/keycloak/tests`](../tests/base/src/test/java/org/keycloak/tests).
When migrating tests, use the remote server mode, as this will make it much quicker to run tests than having to start/stop
Keycloak when you run the test from the IDE.
Add `@KeycloakIntegrationTest` to the class.
### Committing changes
Change `import org.junit.Test;` to `import org.junit.jupiter.api.Test;`
Migrating some tests requires changing more than 50% of the test class. For this reason, git thinks a new file was
created instead of moved. This causes us to lose the history of the test file. To mitigate this, we have a script to make
all commits for you.
Remove extends `AbstractKeycloakTest` as the new test framework provides injection of resources needed by the test there
is no need for the `AbstractKeycloakTest` and tests should instead inject what they need.
One thing your test is most likely going to need is a realm, this is now done with:
```
@InjectRealm
ManagedRealm realm;
In the [`migration-util` module](./migration-util) run:
```shell
./commit-migration.sh
```
### Changed packages/classes
The script works only with your git staging area. If it detects the same files are marked as deleted and created, it makes
one commit where the files are moved unchanged to the new location. Then, it commits the changes with the rest of
your staging area. You will be prompted to provide a commit message if you do not wish to use the default one.
| Old | New |
|---------------------------------------------|------------------------------------------------|
| org.junit.Assert | org.junit.jupiter.api.Assertions |
| org.junit.Test | org.junit.jupiter.api.Test |
| org.keycloak.testsuite.admin.ApiUtil | org.keycloak.tests.utils.admin.ApiUtil |
| org.keycloak.testsuite.util.AdminEventPaths | org.keycloak.tests.utils.admin.AdminEventPaths |
When you create a PR and check the "Files changed" tab, you will still see them as deleted and created, which makes the
code review hard. Instead, go to the "Commits" tab and **review the commit that modifies the already moved file**.
### Assertions
Change `import org.junit.Assert;` to `import org.junit.jupiter.api.Assertions;`, and replace `Assert.` with `Assertions.` throughout.
If the assert passes a message (for example `Assert.assertEquals("Message", expected, actual)`) the message in `Assertions`
is now the last parameter (for example `Assertions.assertEquals(expected, actual, "Message")`).
### Admin events
Admin events are handled slightly differently in the new test framework.
An example for the old testsuite:
```
@Rule
public AssertAdminEvents assertAdminEvents = new AssertAdminEvents(this);
public void myTest() {
assertAdminEvents.assertEvent(realmId, OperationType.CREATE, ...);
}
```
Converted to the new test framework:
```
@InjectAdminEvents
public AdminEvents adminEvents;
public void myTest() {
AdminEventAssertion.assertEvent(adminEvents.poll(), OperationType.CREATE, ...);
}
```
Notice that there is no need to pass `realmId` when asserting an event, that is because the `AdminEvents` will only
receive events for the current realm.
For better readability `AdminEventAssertion` provides a method chaining approach to assert various fields in the event
(the example above could be change to `AdminEventAssertion.assertSuccess(adminEvents.poll()).operationType(OperationType.CREATE)...`).
There is also improved support for skipping events using skip methods, that allows skipping one event (`.skip()`),
multiple events (`.skip(5)`), or skipping all previous events (`.skipAll()`).
**Do not squash the commits** when merging the PR, or the file history will be lost again.

21
tests/MOVING_TESTS.md
View File

@@ -1,21 +0,0 @@
# Instructions for moving and refactoring tests to the new testsuite
When moving and refactoring tests from the old testsuite to the new one it might happen that, once the changes are commited,
the git history of the file is lost. This is due to the way that git internally handles file renames.
In order to preserve the file history, we have come up with a procedure that will work as the following describes.
Let's assume we are migrating `SampleTest.java` to the new testsuite.
1. Move the file `SampleTest.java` (without modifying the content) to the new location using any method you like (`mv`, `git mv`, cut and paste, etc.).
2. Commit that movement using a commit message like `Move SampleTest.java to the new testsuite`.
3. Do all the necessary changes and refactors in the file for using the new testing framework.
4. Commit the refactoring using a commit message like `Refactor SampleTest to use the new testing framework`.
5. Push the changes and create a pull request that will contain both commits.
Once the pull request is created, it might happen that in the `Files changed` tab we see one file deleted
and a new one (with the refactored code) created. This can make difficult the code review, since we don't see the differences
with the previous code.
For seeing the changes as in a usual pull request go to the `Commits (2)` tab and select the commit that refactors the code. Comments,
reviews and conversations can be added here and will be visible in the rest of the pull request sections.

23
tests/migration-util/src/main/java/org/keycloak/test/migration/CommonStatementsRewrite.java Normal file
View File

@@ -0,0 +1,23 @@
package org.keycloak.test.migration;
import java.util.Map;
public class CommonStatementsRewrite extends TestRewrite {
private final Map<String, String> STATEMENTS = Map.of(
"testRealmResource()", "managedRealm.admin()"
);
@Override
public void rewrite() {
for (int i = 0; i < content.size(); i++) {
String l = content.get(i);
for (Map.Entry<String, String> entry : STATEMENTS.entrySet()) {
if (l.contains(entry.getKey())) {
replaceLine(i, l.replace(entry.getKey(), entry.getValue()));
info(i, "Statement rewritten: '" + entry.getKey() + "' --> '" + entry.getValue() + "'");
}
}
}
}
}

24
tests/migration-util/src/main/java/org/keycloak/test/migration/MigrateTest.java
View File

@@ -5,12 +5,18 @@ import java.io.BufferedWriter;
import java.io.FileReader;
import java.io.FileWriter;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.LinkedList;
import java.util.List;
import java.util.stream.Stream;
import static java.lang.Thread.sleep;
public class MigrateTest {
private static final String DIFF_COMMAND = System.getenv("DIFFTOOL");
@@ -22,7 +28,8 @@ public class MigrateTest {
UpdateAssertsRewrite.class,
AddManagedResourcesRewrite.class,
AdminEventAssertRewrite.class,
BeforeRewrite.class);
BeforeRewrite.class,
CommonStatementsRewrite.class);
Path rootPath = getRootPath();
Path oldTestsuitePath = rootPath.resolve("testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite").toAbsolutePath();
@@ -66,9 +73,20 @@ public class MigrateTest {
writeFile(content, destinationPath);
if (DIFF_COMMAND != null && !DIFF_COMMAND.isEmpty()) {
List<String> args = new ArrayList<>(List.of(DIFF_COMMAND.split(" ")));
args.add(testPath.toString());
args.add(destinationPath.toString());
ProcessBuilder pb = new ProcessBuilder();
pb.command(DIFF_COMMAND, testPath.toString(), destinationPath.toString());
pb.start();
pb.command(args);
Process diffProcess = pb.start();
BufferedReader diffOutput = new BufferedReader(new InputStreamReader(diffProcess.getInputStream()));
String line = diffOutput.readLine();
while (line != null) {
System.out.println(line);
line = diffOutput.readLine();
}
diffOutput.close();
}
}

21
tests/migration-util/src/main/java/org/keycloak/test/migration/RenameImportsRewrite.java
View File

@@ -6,20 +6,37 @@ public class RenameImportsRewrite extends TestRewrite {
Map<String, String> IMPORTS = Map.of(
"org.junit.Assert", "org.junit.jupiter.api.Assertions",
"org.keycloak.testsuite.Assert", "org.keycloak.tests.utils.Assert",
"org.junit.Test", "org.junit.jupiter.api.Test",
"org.keycloak.testsuite.util.AdminEventPaths", "org.keycloak.tests.utils.admin.AdminEventPaths",
"org.keycloak.testsuite.admin.ApiUtil", "org.keycloak.testframework.util.ApiUtil"
);
Map<String, String> STATIC_IMPORTS = Map.of(
"org.junit.Assert", "org.junit.jupiter.api.Assertions"
);
@Override
public void rewrite() {
for (int i = 0; i < findClassDeclaration(); i++) {
String l = content.get(i);
if (l.startsWith("import ")) {
if (l.startsWith("import static ")) {
String current = l.substring("import static ".length(), l.lastIndexOf('.'));
String method = l.substring(l.lastIndexOf('.'), l.length() - 1);
String migrateTo = STATIC_IMPORTS.get(current);
if (migrateTo != null) {
replaceLine(i, l.replace(current, migrateTo));
info(i, "Static import rewritten: '" + current + method + "' --> '" + migrateTo + method + "'");
}
} else if (l.startsWith("import ")) {
String current = l.substring("import ".length(), l.length() - 1);
String migrateTo = IMPORTS.get(current);
if (migrateTo != null) {
replaceLine(i, "import " + migrateTo + ";");
replaceLine(i, l.replace(current, migrateTo));
info(i, "Import rewritten: '" + current + "' --> '" + migrateTo + "'");
}

5
tests/migration-util/src/main/java/org/keycloak/test/migration/TestRewrite.java
View File

@@ -44,14 +44,19 @@ public abstract class TestRewrite {
String add = "import " + clazzName + ";";
int l = -1;
int lastImport = -1;
for (int i = 0; i < content.size(); i++) {
String c = content.get(i);
if (c.matches("import [^ ]*;")) {
lastImport = i;
if (c.compareTo(add) > 1) {
l = i;
break;
}
} else if (c.matches("^\\b(?:public\\s+)?class\\b.*Test\\s+\\{$")) {
l = lastImport + 1;
break;
}
}

53
tests/migration-util/src/main/java/org/keycloak/test/migration/UpdateAssertsRewrite.java
View File

@@ -6,29 +6,52 @@ import java.util.List;
public class UpdateAssertsRewrite extends TestRewrite {
private final static String FAIL = "fail";
private final static String ASSERT_THROWS = "assertThrows";
private final static String ASSERT_TRUE = "assertTrue";
private final static String ASSERT_FALSE = "assertFalse";
private final static String ASSERT_NULL = "assertNull";
private final static String ASSERT_NOT_NULL = "assertNotNull";
private final static String ASSERT_EQUALS = "assertEquals";
private final static String ASSERT_NOT_EQUALS = "assertNotEquals";
@Override
public void rewrite() {
for (int i = 0; i < content.size(); i++) {
String l = content.get(i);
String trimmed = l.trim();
if (trimmed.startsWith("Assert.")) {
if (trimmed.startsWith("Assert.") && trimmed.endsWith(";")) {
String method = trimmed.substring("Assert.".length(), trimmed.indexOf("("));
int arguments = l.substring(l.indexOf("(") + 1, l.lastIndexOf(")")).split(", ").length;
if (method.equals("fail")) {
directReplace(i, "fail");
} else if (method.equals("assertTrue") && arguments == 1) {
directReplace(i, "assertTrue");
} else if (method.equals("assertNull") && arguments == 1) {
directReplace(i, "assertNull");
} else if (method.equals("assertNotNull") && arguments == 1) {
directReplace(i, "assertNotNull");
} else if (method.equals("assertNotNull") && arguments == 2) {
moveMessageToLast(i, "assertNotNull");
} else if (method.equals("assertEquals") && arguments == 2) {
directReplace(i, "assertEquals");
} else if (method.equals("assertEquals") && arguments == 3) {
moveMessageToLast(i, "assertEquals");
if (method.equals(FAIL)) {
directReplace(i, FAIL);
} else if (method.equals(ASSERT_THROWS) && arguments == 3) {
moveMessageToLast(i, ASSERT_THROWS);
} else if (method.equals(ASSERT_TRUE) && arguments == 1) {
directReplace(i, ASSERT_TRUE);
} else if (method.equals(ASSERT_TRUE) && arguments == 2) {
moveMessageToLast(i, ASSERT_TRUE);
} else if (method.equals(ASSERT_FALSE) && arguments == 1) {
directReplace(i, ASSERT_FALSE);
} else if (method.equals(ASSERT_FALSE) && arguments == 2) {
moveMessageToLast(i, ASSERT_FALSE);
} else if (method.equals(ASSERT_NULL) && arguments == 1) {
directReplace(i, ASSERT_NULL);
} else if (method.equals(ASSERT_NULL) && arguments == 2) {
moveMessageToLast(i, ASSERT_NULL);
} else if (method.equals(ASSERT_NOT_NULL) && arguments == 1) {
directReplace(i, ASSERT_NOT_NULL);
} else if (method.equals(ASSERT_NOT_NULL) && arguments == 2) {
moveMessageToLast(i, ASSERT_NOT_NULL);
} else if (method.equals(ASSERT_EQUALS) && arguments == 2) {
directReplace(i, ASSERT_EQUALS);
} else if (method.equals(ASSERT_EQUALS) && arguments == 3) {
moveMessageToLast(i, ASSERT_EQUALS);
} else if (method.equals(ASSERT_NOT_EQUALS) && arguments == 2) {
directReplace(i, ASSERT_NOT_EQUALS);
} else if (method.equals(ASSERT_NOT_EQUALS) && arguments == 3) {
moveMessageToLast(i, ASSERT_NOT_EQUALS);
}
}
}