[SPARK-58852][INFRA] Replace archived test report action

[gaogaotiantian]

What changes were proposed in this pull request?

Replace scacap/action-surefire-report with EnricoMi/publish-unit-test-result-action for test reporting.

Why are the changes needed?

scacap/action-surefire-report is archived and removed from apache's allowlist. It has a descendent but none of them are used as much as EnricoMi/publish-unit-test-result-action , which is already listed on apache's allowlist. We should use the popular projects which is better maintained and tested. Also it saves us trouble to create a new entry for the allowed action.

We considered dorny/test-reporter but we realized that the latest released version (v3.0.0) does not have the list-files options, which means it will always list all the files in the test suite and it will always overflow the buffer github allows.

Does this PR introduce any user-facing change?

No, CI only.

How was this patch tested?

CI should show this report. test report action on apache/spark has been failing for a while.

Was this patch authored or co-authored using generative AI tooling?

No.

[dongjoon-hyun]

Thank you for working on this, and the motivation is valid: the Report test results runs on apache/spark are currently all startup_failure because scacap/action-surefire-report is no longer on the ASF allowlist. The pinned SHA a43b3a5 is dorny/test-reporter v3.0.0 and it is listed in apache/infrastructure-actions/approved_patterns.yml, so that part is good.

However, I believe there are two blockers in the current diff that would leave us with no test report at all.

1. check_name is not an input of dorny/test-reporter; the input is name, and it is required.

From action.yml@a43b3a5:

  name:
    description: Name of the check run
    required: true

There is no default, and src/main.ts reads it as:

readonly name = core.getInput('name', {required: true})

So check_name is silently ignored as an unexpected input and the action fails at startup with Input required and not supplied: name.

2. In v3, use-actions-summary defaults to true, which means no check run is created.

createReport() in src/main.ts takes a different branch depending on this input:

if (this.useActionsSummary) {
  ...
  await core.summary.addRaw(summary).write()
} else {
  core.info(`Creating check run ${name}`)
  const createResp = await this.octokit.rest.checks.create({...})
  ...
  const annotations = getAnnotations(results, this.maxAnnotations)
  ...
}

With the default (true), the report is only written to the job summary of the Report test results workflow run. No check run is attached to the commit and no annotations are created, so the report would no longer show up on the PR, and tooling that reads .../check-runs/<id>/annotations to triage CI failures would stop working. We need use-actions-summary: false to keep the current behavior.

Putting both together:

    - name: Publish test report
      uses: dorny/test-reporter@a43b3a5f7366b97d083190328d2c652e1a8b6aa2
      with:
        name: Report test results
        reporter: java-junit
        token: ${{ secrets.GITHUB_TOKEN }}
        path: "**/target/test-reports/*.xml"
        use-actions-summary: false
        fail-on-empty: false

One note on verification: since workflow_run-triggered workflows always run the workflow file from the repository's default branch, this change cannot be exercised by this PR's CI, and it would first execute only after it lands on master. Could you temporarily push this to master of your fork and trigger a Build run there, then link the resulting run here? Given the two issues above, it would be good to confirm the report is actually produced before merging.

[gaogaotiantian]

I pushed the code to my own master as @dongjoon-hyun suggested, and the test report worked fine - https://github.com/gaogaotiantian/spark/runs/96371809832 . The job itself is https://github.com/gaogaotiantian/spark/actions/runs/32351605789/job/96371761596 .

[gaogaotiantian]

After some investigation, I realized that dorny/test-reporter v3 is probably not applicable in our case. It does not have filters to make sure only failed tests are displayed - which means it will always overflow the 65k buffer for action report. I'm investigating some other options. For now, converting this to draft.

[gaogaotiantian]

The new action looks promising - https://github.com/gaogaotiantian/spark/runs/96782026558

I will update the description and revert the intentional test failure later, but this action seems to be a good alternative for our old one. Also it's listed on ASF's allowlist already.

[gaogaotiantian]

Hi all, I've updated the description and made the PR ready for review. The example report is listed above, which I obtained by changing my master branch with the action proposed. If you have any other question, please let me know.

[gaogaotiantian]

Merge Summary:

• merged into master 05c2c12
• merged into branch-4.x 9002a1c

Posted by merge_spark_pr.py