dongjoon-hyun commented on PR #58098: URL: https://github.com/apache/spark/pull/58098#issuecomment-5345557056
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`](https://github.com/dorny/test-reporter/blob/a43b3a5f7366b97d083190328d2c652e1a8b6aa2/action.yml): ```yaml name: description: Name of the check run required: true ``` There is no default, and [`src/main.ts`](https://github.com/dorny/test-reporter/blob/a43b3a5f7366b97d083190328d2c652e1a8b6aa2/src/main.ts) reads it as: ```ts 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: ```ts 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: ```yaml - 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. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
