search

lorenzwalthert / touchstone

Smart benchmarking of pull requests with statistical confidence
https://lorenzwalthert.github.io/touchstone
Other
53 stars 7 forks source link

Fix how the author association is looked up for PRs. #135

Closed plietar closed 3 months ago

plietar commented 3 months ago

The use_touchstone_workflows was generating code that used the github.event.comment.author_association regardless of the trigger type. However this does not work for pull_request triggers, meaning the if condition would always fail to evaluate and the workflow never ran. This commit fixes that by using the correct field name.

plietar commented 3 months ago

See https://github.com/plietar/touchstone-test/actions/runs/9128401168/workflow for a workflow run that got skipped, even though it was opened by the repo owner.

lorenzwalthert commented 3 months ago

Thanks. Did you test this PR already as the other one with your workflow? I think we also lack proper integration tests in this repo. Would be a great addition so we can merge non-trivial, hard to test changes like this one with more confidence.

plietar commented 3 months ago

Yes this workflow run uses the change: https://github.com/plietar/touchstone-test/actions/runs/9128351202 It got triggered by opening the PR.

(~The workflow got stuck when installing dependencies for some reason I'm still trying to debug, but that should be unrelated~ I updated the OS version and RSPM and that solved my workflows getting stuck)

I agree integration tests would be nice, but I don't really know if we can run actions as part of the CI. In the meantime the snapshots are at least a good way to see what gets produced as a workflow file.

assignUser commented 3 months ago

Ah good catch, thanks.

if we can run actions as part of the CI

Yes you can run repo-local actions via - uses: ./path/to/action here is an example. Proper tests would be great!