Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Lexical] Add label to run e2e tests on approve #6120

Merged
merged 20 commits into from
May 16, 2024
24 changes: 24 additions & 0 deletions .github/workflows/add-label-run-extended-tests-approve.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
name: Add Label to run extended tests on approve
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The file name of this workflow should be 'add-label-after-approved'. This workflow shouldn't be running e2e tests, just adding a label


on:
pull_request_review:

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why cancel in progress?


jobs:
label_pull_requests:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The naming format used for all other workflows is foo-bar. So this one should probably be called something like label-after-approved

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback, will update the naming conventions

if: github.event.review.state == 'approved' && !contains( github.event.pull_request.labels.*.name, 'extended-tests')
runs-on: ubuntu-latest
permissions:
pull-requests: write
steps:
- run: gh pr edit "$NUMBER" --add-label "extended-tests"
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GH_REPO: ${{ github.repository }}
NUMBER: ${{ github.event.pull_request.number }}
e2e-tests:
needs: label_pull_requests
uses: ./.github/workflows/call-e2e-all-tests.yml
Comment on lines +22 to +24
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is right. Once the label is added, it should kick off the other tests-extended workflow automatically, as that already gets triggered when labels are added.

Copy link
Contributor Author

@Sahejkm Sahejkm May 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this and found out that 1 GitHub workflow action can't trigger another workflow action and hence this was needed, with a "needs" check , the second job won't run unless 1 job is not skipped. Without this tests don't trigger on approve unless there is a manual action, other way to fix this is use PAT tokens to trigger the 1st job to add labels,but I think we should still use the GitHub action bot.

3 changes: 1 addition & 2 deletions .github/workflows/tests-extended.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
name: Lexical Tests (Extended)

on:
merge_group:
pull_request:
types: [labeled, synchronize]
paths-ignore:
Expand All @@ -13,5 +12,5 @@ concurrency:

jobs:
e2e-tests:
if: (github.repository_owner == 'facebook' && contains(github.event.pull_request.labels.*.name, 'extended-tests')) || github.event_name == 'merge_group'
if: github.repository_owner == 'facebook' && contains(github.event.pull_request.labels.*.name, 'extended-tests')
uses: ./.github/workflows/call-e2e-all-tests.yml
Loading