XComp commented on code in PR #21736:
URL: https://github.com/apache/flink/pull/21736#discussion_r1190087803
##########
flink-test-utils-parent/flink-migration-test-utils/README.md:
##########
@@ -0,0 +1,99 @@
+# Add State Migration Tests
+
+This module collects tools that help to generate test data for the state
migration tests.
+
+The following dependency need to be added to the module's Maven config in case
a
+migration test is meant to be added to that module:
+
+```xml
+<dependency>
+ <groupId>org.apache.flink</groupId>
+ <artifactId>fink-migration-test-utils</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+</dependency>
+```
+
+and the following profile
+
+```xml
+<profile>
+ <id>generate-migration-test-data</id>
+ <build>
+ <plugins>
+ <plugin>
+ <artifactId>maven-antrun-plugin</artifactId>
+ <executions>
+ <execution>
+ <id>generate-migration-test-data</id>
+ <phase>package</phase>
+ <goals>
+ <goal>run</goal>
+ </goals>
+ <configuration>
+ <target>
+ <condition property="optional.classes"
value="--classes '${generate.classes}'"
+ else="">
+ <isset property="generate.classes"/>
+ </condition>
+ <condition property="optional.prefixes"
+ value="--prefixes
'${generate.prefixes}'" else="">
+ <isset property="generate.prefixes"/>
+ </condition>
+ <java
classname="org.apache.flink.test.migration.MigrationTestsSnapshotGenerator"
+ fork="true" failonerror="true"
dir="${project.basedir}">
+ <classpath refid="maven.test.classpath"/>
+ <arg value="--dir"/>
+ <arg line="${project.basedir}"/>
+ <arg value="--version"/>
+ <arg value="${generate.version}"/>
+ <arg line="${optional.classes}"/>
+ <arg line="${optional.prefixes}"/>
+ </java>
+ </target>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </build>
+</profile>
+```
+
+To show the log during generating, add
+
+```
+logger.migration.name = org.apache.flink.test.migration
+logger.migration.level = INFO
+```
+
+to the `log4j2-test.properties` of this module.
+
+The state migration tests should satisfy
+
+1. The tests are named like `*(Test|ITCase).(java|scala)`.
+2. The test class name is the same with the file name.
+3. The test implements `org.apache.flink.test.util.MigrationTest` and the
snapshots generator methods are labeled
+ with `@SnapshotsGenerator` or `@ParameterizedSnapshotsGenerator`.
+
+# Generating Snapshots
+
+To generate the snapshots for all the tests,
+execute from within the target version's release branch:
+
+```shell
+mvn clean package -Pgenerate-migration-test-data -Dgenerate.version=1.17 -nsu
-Dfast -DskipTests
+```
+
+The version (`1.17` in the command above) should be replaced with the target
one.
+
+By default, it will search for the migration tests under `src/test/java` and
`src/test/scala`. It is also supported
+to change the default search paths or only generating for specific classes:
Review Comment:
```suggestion
to change the default search paths or only generate for specific classes:
```
##########
flink-tests/src/test/java/org/apache/flink/test/state/operator/restore/AbstractOperatorRestoreTestBase.java:
##########
@@ -104,16 +121,33 @@ public abstract class AbstractOperatorRestoreTestBase
extends TestLogger {
.setNumberSlotsPerTaskManager(NUM_SLOTS_PER_TM)
.build());
- private final boolean allowNonRestoredState;
private final ScheduledExecutor scheduledExecutor =
new
ScheduledExecutorServiceAdapter(EXECUTOR_RESOURCE.getExecutor());
- protected AbstractOperatorRestoreTestBase() {
- this(true);
+ protected AbstractOperatorRestoreTestBase(FlinkVersion flinkVersion) {
+ this.flinkVersion = flinkVersion;
}
- protected AbstractOperatorRestoreTestBase(boolean allowNonRestoredState) {
- this.allowNonRestoredState = allowNonRestoredState;
+ protected void internalGenerateSnapshots(FlinkVersion targetVersion)
throws Exception {
+ ClusterClient<?> clusterClient = cluster.getClusterClient();
+ final Deadline deadline = Deadline.now().plus(TEST_TIMEOUT);
+
+ // Submit job with old version savepoint and create a migrated
savepoint in the new version.
+ // Any old version is ok, and we choose 1.8 directly.
+ String savepointPath = migrateJob(FlinkVersion.v1_8, clusterClient,
deadline);
+
+ Path targetPath = getSavepointPath(targetVersion);
+ Files.createDirectories(targetPath);
+
+ // copy the savepoint to the give directory
Review Comment:
```suggestion
// copy the savepoint to the given directory
```
nit
##########
flink-annotations/src/main/java/org/apache/flink/FlinkVersion.java:
##########
@@ -87,7 +87,11 @@ public static Optional<FlinkVersion> byCode(String code) {
return Optional.ofNullable(CODE_MAP.get(code));
}
- /** Returns the current version. */
+ public static FlinkVersion valueOf(int majorVersion, int minorVersion) {
+ return FlinkVersion.valueOf("v" + majorVersion + "_" + minorVersion);
+ }
+
+ /** Returns the current master version. */
Review Comment:
```suggestion
/** Returns the version for the current branch. */
```
nit: "current master" is not correct because this code will end up in
release branches as well eventually where we don't update the FlinkVersion.
WDYT?
##########
flink-test-utils-parent/flink-migration-test-utils/README.md:
##########
@@ -0,0 +1,99 @@
+# Add State Migration Tests
+
+This module collects tools that help to generate test data for the state
migration tests.
+
+The following dependency need to be added to the module's Maven config in case
a
Review Comment:
```suggestion
The following dependency needs to be added to the module's Maven config in
case a
```
nit
##########
flink-tests/src/test/java/org/apache/flink/test/state/operator/restore/AbstractOperatorRestoreTestBase.java:
##########
@@ -71,7 +80,15 @@
* AbstractOperatorRestoreTestBase#migrateJob}, please create the
corresponding test resource
* directory and copy the _metadata file by hand.
Review Comment:
this paragraph is obsolete
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
