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

chore(github-workflow): run the tests only when the tests belong to t… #210

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MitanOmar
Copy link
Member

…he changed folder

@MitanOmar MitanOmar force-pushed the run-tests-only-when-needed branch from ab50cc8 to 8839c3a Compare May 23, 2024 15:33
@@ -4,8 +4,12 @@ name: Chart
on:
push:
branches: [main]
paths:
- 'charts/**'
Copy link
Contributor

Choose a reason for hiding this comment

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

the chart tests also build and install the container images so these tests could in theory also fail if we change things in other paths

as such they currently seem to serve as simple full stack smoketests, maybe we want to consider keeping them on all changes for that purpose?

Copy link
Member Author

Choose a reason for hiding this comment

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

i remove the condition from the charts workflow, but why i see that the tests are still Waiting for status to be reported

Copy link
Member Author

Choose a reason for hiding this comment

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

@hairmare do you know why there is three frontend tests are there even if we define the job to be executed only if there is a change in the frontend folder ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

our rules still need them to succeed

Copy link
Collaborator

Choose a reason for hiding this comment

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

image

Copy link
Member Author

Choose a reason for hiding this comment

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

i will keep this PR open, since there are other tasks more important than this, we can come to it later

Copy link
Collaborator

Choose a reason for hiding this comment

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

# without paths
...
jobs:
  dirty:
     outputs:
       dirty: ${{ steps.check-dirty.outputs.dirty }}
     steps:
      - id: dirty
        run: |
          # bash script to determine if dirty, then write to output

  lint:
      needs: dirty
      steps:
       - name: foo
         if: ${{ needs.dirty.outputs.dirty }}
         run: |
           ls -a

something like this would probably work, but is very ugly

Copy link
Member Author

Choose a reason for hiding this comment

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

i totally agree that it's ugly

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if it works, but could you try the inverse of paths.

  • For the backend workflow:
  pull_request:
    paths-ignore:
      - 'frontend/**'
  • For the frontend workflow:
  pull_request:
    paths-ignore:
      - 'backend/**'

Copy link
Contributor

Choose a reason for hiding this comment

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

bump

@MitanOmar MitanOmar force-pushed the run-tests-only-when-needed branch from 8839c3a to cef46e2 Compare May 23, 2024 15:38
@MitanOmar MitanOmar force-pushed the run-tests-only-when-needed branch from cef46e2 to 4976db6 Compare May 23, 2024 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants