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.

Sign up for GitHub

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

Jump to bottom

[Lexical][CI] Fix issues in after_approval workflow #6307

Merged
merged 10 commits into from
Jun 14, 2024
Merged

[Lexical][CI] Fix issues in after_approval workflow #6307

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9b904ad
Fix issues in after_approval workflow
Sahejkm Jun 14, 2024
7f23cde
donot run flaky-tests-notice on PRs from fork
Sahejkm Jun 14, 2024
5dedb8e
Fix after testing
Sahejkm Jun 14, 2024
eb1ef4a
Fix after testing
Sahejkm Jun 14, 2024
04a0d18
.
Sahejkm Jun 14, 2024
9f63f5b
.
Sahejkm Jun 14, 2024
7d06644
.
Sahejkm Jun 14, 2024
c181d7f
test skip
Sahejkm Jun 14, 2024
41a186b
test
Sahejkm Jun 14, 2024
8e1ec99
remove flaky-test-notice till it cud be fixed
Sahejkm Jun 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 3 additions & 14 deletions .github/workflows/after-approval.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,23 @@ on:

jobs:
pre_job:
if: (github.event.review.state == 'approved' && !contains(github.event.pull_request.labels.*.name, 'extended-tests'))
runs-on: ubuntu-latest
outputs:
should_skip: ${{ steps.skip_check.outputs.should_skip }}
steps:
- id: skip_check
uses: fkirc/skip-duplicate-actions@v5
with:
concurrent_skipping: 'always'
concurrent_skipping: 'same_content_newer'
skip_after_successful_duplicate: 'true'
do_not_skip: '["pull_request", "merge_group"]'
paths-ignore: '["packages/lexical-website/**", "packages/*/README.md", ".size-limit.js"]'
paths_ignore: '["packages/lexical-website/**", "packages/*/README.md", ".size-limit.js"]'
e2e-tests:
needs: pre_job
if: needs.pre_job.outputs.should_skip != 'true' && (github.event.review.state == 'approved' && !contains(github.event.pull_request.labels.*.name, 'extended-tests'))
uses: ./.github/workflows/call-e2e-all-tests.yml

flaky-test-notice:
Copy link
Contributor

Choose a reason for hiding this comment

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

can we please leave this here first? it still works on branches on this repo. ill like to try and fix instead of deleting it so quickly

Copy link
Contributor Author

@Sahejkm Sahejkm Jun 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

  • this keeps the pipeline red and its not a new issue we are trying to fix, its kinda wrong signal to PRs from forks, lemme check if we can figure out if PR is from fork else i would like to not make the CI red

  • we can readd this once we have the fix ready, i have tried multiple approaches in the past for the "addLabel" workflow with same issue but it didn't seem to work that time, so i don't think its a quick fix

Copy link
Contributor

Choose a reason for hiding this comment

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

lemme check if we can figure out if PR is from fork else i would like to not make the CI red

this would be great, if its failing the ci and cant figure something out for the above then im ok to remove first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@potatowagon i tried couple if approaches but looks like "always()" is causing this step to always run regardless of whatever if else check i put related to ${{ github.event.pull_request.head.repo.full_name != 'facebook/lexical' }} anywhere in code.

would prefer removing this 1st till we get a better fix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

check this weird run

https://github.com/facebook/lexical/actions/runs/9510521267/job/26215150424

Screenshot 2024-06-14 at 12 55 08 PM

it evaluating (true && (needs.e2e)) as true

needs: e2e-tests
runs-on: ubuntu-latest
if: always()
steps:
- name: Flaky test notice PR comment
if: needs.e2e-tests.result == 'failure' || needs.e2e-tests.result == 'cancelled'
uses: thollander/actions-comment-pull-request@v2
with:
message: |
E2E tests failed on [this run](https://github.com/facebook/lexical/actions/runs/${{ github.run_id }}). Please check if the failure is due to a flaky test. Visit the [Flaky Test Tracker](https://github.com/facebook/lexical/discussions/6289) for known flaky tests, and record the failed test run there if its a flaky failure.

integration-tests:
needs: pre_job
if: needs.pre_job.outputs.should_skip != 'true' && (github.event.review.state == 'approved' && !contains(github.event.pull_request.labels.*.name, 'extended-tests'))
Expand Down
12 changes: 0 additions & 12 deletions .github/workflows/tests-extended.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,6 @@ jobs:
if: github.repository_owner == 'facebook' && contains(github.event.pull_request.labels.*.name, 'extended-tests')
uses: ./.github/workflows/call-e2e-all-tests.yml

flaky-test-notice:
needs: e2e-tests
runs-on: ubuntu-latest
if: always()
steps:
- name: Flaky test notice PR comment
if: needs.e2e-tests.result == 'failure' || needs.e2e-tests.result == 'cancelled'
uses: thollander/actions-comment-pull-request@v2
with:
message: |
E2E tests failed on [this run](https://github.com/facebook/lexical/actions/runs/${{ github.run_id }}). Please check if the failure is due to a flaky test. Visit the [Flaky Test Tracker](https://github.com/facebook/lexical/discussions/6289) for known flaky tests, and record the failed test run there if its a flaky failure.

integration-tests:
if: github.repository_owner == 'facebook' && contains(github.event.pull_request.labels.*.name, 'extended-tests')
uses: ./.github/workflows/call-integration-tests.yml
Loading