Collecting benchmark results in PR is broken

[krzema12]

See e.g. https://github.com/krzema12/snakeyaml-engine-kmp/actions/runs/11074527066/job/30774051979

Error: 'comment-on-alert' is enabled but 'github-token' is not set. Please give API token to send commit comment on alert

[krzema12]

I think it worked some time ago, I recall the comments written by the action.

However, now I noticed that in the problematic case, in the implicit "Set up job" step, GitHub shows Secret source: None, and when this action runs on the main branch, it's Secret source: Actions. Here's a possible explanation: actions/runner#3039 (comment)

[krzema12]

Reading the docs of the benchmark action - they recommend not running the action in pull requests (https://github.com/benchmark-action/github-action-benchmark?tab=readme-ov-file#run-only-on-your-branches) but because of another reason: to avoid publishing benchmark results in PRs. This docs entry, however, is some kind of indication that PRs are not supported or at least tested.

Related, other folks have the same problem: benchmark-action/github-action-benchmark#30

[krzema12]

@OptimumCode do you happen to have capacity to take a look at this? If not, that's cool - I'll try to fix it once I'm back from 🌴 (~2 weeks) 😄

[OptimumCode]

Hi, @krzema12.

The problem is related to the fact that repository's secrets are not available for PRs made from a fork repositories. And you are using a PAT token to upload results into a separate repo.

As a solution (I had the same problem in my repo with other workflows), we can split workflow into two workflows:

• one triggered on pull_request that runs benchmarks and uploads results as job artifacts
• another one that is triggered on workflow_run(secrets are available on this trigger) and uploads the results after the first workflow is finished

Do you have any objections/suggestions/comments regarding the above, @krzema12 ?

I will try to find some time to have a closer look to the issue next week (probably I am missing something here because when I experimented on my repo I didn't not change the benchmark workflow)

[krzema12]

Ah right, it's about forks, not all PRs!
What you propose makes sense, but on the other hand I don't insist on fixing it. I think we could live with how things are right now, maybe with a small change: make the current workflow fail with a clear message if we hit this edge case. WDYT?

[OptimumCode]

We can probably just skip the results upload in this case (add a condition to check whether the source repo owner is different). I think it would be better than a failed job

[OptimumCode]

One more option: we can enable commentOnAlert only when the source repo is the same as the target repo. In this case, we will still have a summary that can be reviewed manually (to check the performance degradation)

[krzema12]

@OptimumCode I like the second idea better!

[OptimumCode]

Hi, @krzema12. I am having some issues with benchmark workflow:

• The ActionsWrapperValidation cannot be resolved
  
• Untyped properties commentOnAlert and autoPush are also not available
  

I believe something might be wrong with the binding server. Could you please advise what the issue could be?

If you point me in the right direction, I think I can look at this issue by myself

[krzema12]

@OptimumCode could you try removing your Maven Local repo, and restarting your IDE?
If it doesn't help, please write on the library's Slack channel, @Vampire could you then take a look as I'm on vacation and short on Internet? 🙏

[OptimumCode]

Thank you, @krzema12. Seems like it helped - the old versions of artifacts were in Maven Local and this was the reason why I had errors.

[OptimumCode]

Seems like everything is working now - the workflow is passed and the summary is available