Azure / Azure/osdu-spi

fix(java-build): run the same Maven phases on pull requests as on push

Closed
#199 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Dockerfile
Stars
1
Forks
9
Avg merge
9h 23m
Merged PRs (30d)
58

Description

## Problem

A pull request can pass Java Build while the push to `main` after its merge fails Java Build on the same tree. danielscholl-osdu/entitlements PR #9 was green; the push run for its merge commit (run 34695861887 at 760f25a) failed with:

```
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-enforcer-plugin:3.6.2:enforce (enforce-test-execution) on project entitlements-v2-azure:
[ERROR] Rule 0: org.opengroup.osdu.EnforceTestExecution failed with message:
[ERROR] The following test methods were found in test classes but were NOT executed by Surefire or Failsafe:
```

The enforcer rule is bound to the `verify` phase in the upstream root pom. Nothing in the pull request diff touched the build; the rule simply never ran on the pull request.

## Cause

`.github/actions/java-build/action.yml` chooses the Maven goals by `generate_coverage`, and `validate.yml` sets that input to true only for `pull_request` and `pull_request_target` events:

```
if [[ "$GENERATE_COVERAGE" == "true" ]]; then
mvn $MAVEN_CLI_OPTS clean package org.jacoco:jacoco-maven-plugin:0.8.11:report
else
mvn $MAVEN_CLI_OPTS clean install
fi
```

`package` stops before `verify`, so any plugin bound to `verify` or `integration-test` (enforcer rules, failsafe, checks a service adds there) runs only on push and dispatch. The pull request lane validates a shorter lifecycle than the one that gates `main`.

## Required change

1. In `.github/actions/java-build/action.yml`, run `mvn $MAVEN_CLI_OPTS clean verify` on the coverage path, keeping the `org.jacoco:jacoco-maven-plugin:0.8.11:report` goal after it. `verify` still produces the JARs docker-build downloads, so the comment on that line about `package` no longer applies and should be dropped.
2. Keep `clean install` on the non-coverage path; `install` already includes `verify`, so both paths now run the same phases through `verify`.
3. Update the java-build action's README (or its header comment, whichever documents the goals) to say both paths run through `verify` and why: a check bound to `verify` in a service pom must gate the pull request, not only `main`.
4. Confirm with the dev-ci workflow that the coverage summary still renders from `target/site/jacoco` after the goal change.

Out of scope: the fork-side cause of the entitlements failure (JUnit 4 tests without a vintage engine in the Azure module), which is fixed in the fork.

Contributor guide

Open the contributing guide

Research direction

Start in .github/actions/java-build/action.yml and compare the coverage and non-coverage Maven commands. Update the documented goals in the java-build README or header comment, then use the dev-ci workflow to confirm the coverage summary still renders from target/site/jacoco; both paths should run through verify and checks bound there should gate pull requests.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
build-system, ci-cd
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
84/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.