sclorg / testing-farm-as-github-action

GitHub Action to execute tests by Testing Farm and update Pull Request status
MIT License
13 stars 11 forks source link

feat: update default value of `git_ref` :safety_pin: #190

Closed jamacku closed 2 months ago

jamacku commented 3 months ago

By default use github.base_ref and use master as a fallback.

This will primarily help when running on pull_request_target.

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 88.38%. Comparing base (60d633f) to head (8fb24e0). Report is 4 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #190 +/- ## ========================================== + Coverage 88.33% 88.38% +0.05% ========================================== Files 10 10 Lines 686 689 +3 Branches 72 73 +1 ========================================== + Hits 606 609 +3 Misses 78 78 Partials 2 2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

LecrisUT commented 3 months ago

Also, you also have access to these variables within the typescript code: https://github.com/actions/toolkit/blob/main/packages/github/src/context.ts#L6-L24

Might be good, because there you can actually check what trigger it was and so on.

jamacku commented 3 months ago

[test]

jamacku commented 3 months ago

[test]

zmiklank commented 2 months ago

I think this is a breaking change. I would consider releasing it with tfaga 3.0 sometime in the future (although 3.0 is not yet even planned).

jamacku commented 2 months ago

I don't think this is a breaking change, but it changes behavior a bit and might surprise some users.

LecrisUT commented 2 months ago

I don't think this is a breaking change, but it changes behavior a bit and might surprise some users.

Initially I thought so too, but it is a breaking change because it changes from master -> github.ref, which at minimum will affect PR builds.

jamacku commented 2 months ago

Initially I thought so too, but it is a breaking change because it changes from master -> github.ref, which at minimum will affect PR builds.

But only in the sense that TF would be using plans from the PR branch, right?

LecrisUT commented 2 months ago

Initially I thought so too, but it is a breaking change because it changes from master -> github.ref, which at minimum will affect PR builds.

But only in the sense that TF would be using plans from the PR branch, right?

As far as I can think of that would be the only implication, but even there that is quite limited for repos that have master as the main branch as that is being discouraged. You may also have discrepancy when it is run on tag.

But, why guess, there are only 28 instances of the action usage: https://sourcegraph.com/search?q=context:global+++%22uses:+sclorg/testing-farm-as-github-action%22+&patternType=keyword&sm=0

phracek commented 2 months ago

For sure I would add it into v3. Let's add it there for sure.

phracek commented 2 months ago

[test]