weiqingy opened a new issue, #1006: URL: https://github.com/apache/flink-agents/issues/1006
### Search before asking - [X] I searched in the [issues](https://github.com/apache/flink-agents/issues) and found nothing similar. ### Description `tools/ut.sh` tells the reader that omitting `-f` tests every supported Flink version. It does not. Omitting `-f` selects one version, and on the Java unit-test path the selected version is not used at all. ## What the help text says `tools/ut.sh:45-48`: ``` -f, --flink Specify Flink version to test (can be used multiple times) Supported versions: 2.3, 2.2, 2.1, 2.0, 1.20 Examples: -f 2.3, -f 1.20, -f 2.3 -f 1.20 Default: run all versions if not specified ``` The examples block repeats it at `tools/ut.sh:53`: ``` $0 --java # Run only Java tests (all Flink versions) ``` `.github/CONTRIBUTING.md:45` introduces the bare form the same way, as "Run all the UT with the following command". ## What the script does A single default is applied when `-f` is omitted. `tools/ut.sh:24` and `tools/ut.sh:110-113`: ```bash DEFAULT_FLINK_VERSION="2.2" ... # If no version is specified, the default version will be run by default. if [ ${#flink_versions[@]} -eq 0 ]; then flink_versions=("${DEFAULT_FLINK_VERSION}") fi ``` So the default is one version, not five. The in-code comment at `:110` already describes the real behavior, which is the opposite of the help text. ## The Java unit-test path ignores the version entirely Without `-e`, the Java half runs `tools/ut.sh:172` and `tools/ut.sh:183`: ```bash mvn --batch-mode --no-transfer-progress test-compile jar:test-jar install -DskipTests ${SPOTLESS_FLAG} ... mvn -T16 --batch-mode --no-transfer-progress test -fae -pl "${exclude_list}" ${SPOTLESS_FLAG} ``` Neither line references `${flink_versions[@]}` and neither passes `-P`, so no Flink profile is activated and the reactor falls back to the root property, `pom.xml:44`: ```xml <flink.version>2.3.0</flink.version> ``` `api`, `plan`, `runtime` and `integrations` all inherit that value, so `-f` has no effect on the Java unit tests at all. `tools/ut.sh -j -f 1.20` compiles and tests against Flink 2.3.0. The E2E path is the contrast case and does honor the flag, at `tools/ut.sh:158`: ```bash mvn --batch-mode --no-transfer-progress test -pl 'e2e-test/flink-agents-end-to-end-tests-integration' -Pflink-${version} ... ``` ## A plain run tests two different Flink lines at once Running `tools/ut.sh` with no flags: - Java resolves Flink `2.3.0` from `pom.xml:44`, since no profile is passed. - Python runs `uv pip install apache-flink~=2.2.0` at `tools/ut.sh:258`, which resolves to PyFlink 2.2.x. So one command tests Java against 2.3.0 and Python against 2.2.x, and neither half is the "all versions" the help promises. ## Where multi-version coverage actually comes from Only the two CI jobs that pass `-e -f` explicitly: - `.github/workflows/ci.yml:205`, `tools/ut.sh -p -e -f ${{ matrix.flink-version }}`, matrix `1.20, 2.0, 2.1, 2.2, 2.3` at `ci.yml:170-182`. - `.github/workflows/ci.yml:246`, `tools/ut.sh -j -e -f ${{ matrix.flink-version }}`, matrix `1.20, 2.0, 2.1, 2.2, 2.3` at `ci.yml:219-231`. Every other invocation is single-version: `ci.yml:128` (`-j`), `ci.yml:156` (`-p`), `build_wheel.yml:64` (`-j`), `build_wheel.yml:90` (`-p`). None passes `-f`. The coverage itself is fine. The problem is that the script claims to provide it and the flag that would provide it is a no-op on the Java unit-test path. ## Why this matters A contributor reading the help reasonably believes a local `tools/ut.sh` covers all five supported versions, and that `-f 1.20` narrows the Java unit tests to 1.20. Neither is true, so a version-specific Java break can pass locally and surface only in the E2E matrix. ## Options Roughly three, and the choice is the maintainers' to make. - Correct the help text and `.github/CONTRIBUTING.md` to describe the single default, and say plainly that `-f` applies to the E2E and Python paths only. - Make the default actually iterate the supported versions, which means giving the Java unit-test path a per-version invocation and accepting the added runtime. - Make `-f` an error, or a warning, on the Java unit-test path so a value that will be ignored is not silently accepted. ### Are you willing to submit a PR? - [X] I'm willing to submit a PR! -- 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]
