krickert commented on code in PR #1120: URL: https://github.com/apache/opennlp/pull/1120#discussion_r3481044137
########## .github/workflows/maven.yml: ########## Review Comment: Found a potential issue: the shell tests may be skipped when the build matrix has partial failures. This doesn't look intentional.. The three new shell-test jobs (`test-unix-shell-ubuntu`, `test-unix-shell-macos`, `test-windows-shell`) all declare: ```yaml needs: build ``` My understanding: The `build` job is a matrix of 6 combinations (`ubuntu`/`macos`/`windows` × `java 21`/`25`) with `fail-fast: false`. In GitHub Actions, `needs: <matrix-job>` waits for the entire matrix to succeed. If any single leg fails (even an unrelated one like `windows-latest` + Java 25), the shell-test jobs are marked as skipped—even though the `ubuntu-latest`/`java 21` leg that produces the `opennlp-distribution` artifact succeeded and uploaded it. The comments in the workflow (lines 60-62 and 74-75) indicate the intent is to reuse the distribution from the Linux/JDK 21 build. This skipping behavior is probably unintentional? Assuming this is the intent, I can think of two ways to fix this: - Introduce a separate, non-matrix job (e.g., `build-distribution`) that only runs on `ubuntu-latest` + Java 21, uploads the artifact, and have the shell tests depend on that job instead. - Or keep the current matrix but make the shell-test dependency more granular (e.g., via an `if:` condition that checks whether the required artifact exists). Thoughts? -- 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]
