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

fix: skip unnecessary test runs for bot-generated and non-critical PRs #1339

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
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
29 changes: 27 additions & 2 deletions .github/workflows/pr-testing-with-test-project.yml
Original file line number Diff line number Diff line change
@@ -1,13 +1,37 @@
#This workflow runs the tests in the test projects to make sure the generator works as a library where it is a Node dependency along with the template.
name: Test using test project

on:
pull_request:
types: [opened, reopened, synchronize, ready_for_review]
paths:
- 'package.json'
- 'package-lock.json'
- 'apps/'
- 'packages/'
- '.github/workflows/'
- '!/*.md'
- '!docs/**'

jobs:
should-test:
runs-on: ubuntu-latest
outputs:
run_tests: ${{ steps.check.outputs.run_tests }}
steps:
- name: Check if tests should run
id: check
run: |
# Skip for bot PRs with specific titles
if [[ "${{ github.actor }}" == "asyncapi-bot" && "${{ github.event.pull_request.title }}" =~ ^(ci:\ update|chore\(release\):) ]] || \
[[ "${{ github.actor }}" == "allcontributors[bot]" && "${{ github.event.pull_request.title }}" =~ ^docs: ]]; then
echo "run_tests=false" >> $GITHUB_OUTPUT
else
echo "run_tests=true" >> $GITHUB_OUTPUT
fi

test:
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding a new job on which the actual job to be done is dependent won't increase job setup time?

Instead, how's about adding an if clause just like it is done here

Copy link
Author

Choose a reason for hiding this comment

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

I did tried adding but thought of optimising it a bit thus made it likewise ,anyway will look into it .

if: github.event.pull_request.draft == false
needs: should-test
if: needs.should-test.outputs.run_tests == 'true' && github.event.pull_request.draft == false
name: Test generator as dependency with Node ${{ matrix.node }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if: needs.should-test.outputs.run_tests == 'true' && github.event.pull_request.draft == false
if: needs.should-test.outputs.run_tests == true && github.event.pull_request.draft == false

wrapping true in single quotes makes it string, although eventually, it results in a true statement (as strings are also considered to be true) so the workflow works as expected. But it's a good practice to use Booleans for truthy and falsy statements.

Copy link
Author

Choose a reason for hiding this comment

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

Alright got it !! i took the assumptions so did such changes but anyway will resolve it .

runs-on: ubuntu-latest
strategy:
Expand All @@ -16,6 +40,7 @@ jobs:
steps:
- name: Checkout repository
uses: actions/checkout@v3

- name: Run test
run: NODE_IMAGE_TAG=${{ matrix.node }} docker compose up --abort-on-container-exit --remove-orphans --force-recreate
working-directory: ./apps/generator/test/test-project
Loading