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

add a check to make sure unit tests are considered when changing code #10337

Merged
merged 4 commits into from
Jun 21, 2024
Merged

add a check to make sure unit tests are considered when changing code #10337

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
38c3a8a
add a check to make sure unit tests are considered when changing code
ChenyuLInx Jun 18, 2024
df46e01
use new action
ChenyuLInx Jun 18, 2024
442d0b1
switch to better config codecove
ChenyuLInx Jun 21, 2024
e06ed29
add back project level
ChenyuLInx Jun 21, 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
39 changes: 4 additions & 35 deletions .github/workflows/unittest-existence.yml
View file
Open in desktop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should refactor https://github.com/dbt-labs/actions/blob/main/.github/workflows/changelog-existence.yml#L2 a bit to make it more generalized and use it here.
@emmyoop WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dbt-labs/actions#210

Original file line number Diff line number Diff line change
Expand Up @@ -35,43 +35,12 @@ permissions:
contents: read
pull-requests: write

env:
comment: "Thank you for your pull request! We could not find a unittest file for this change. Please consider adding some. If it is hard to unittest, please add the 'Hard to Unit Test' label. If unit tests are not needed, please add the 'Unit Test Not Needed' label."


jobs:
unittest:
name: Check if Unit Test being added.
if: ${{ !contains(github.event.pull_request.labels.*.name, 'Unit Test Not Needed') && !contains(github.event.pull_request.labels.*.name, 'Hard to Unit Test')}}
runs-on: ubuntu-latest
steps:
- name: Check if unittest file was added
uses: dorny/paths-filter@v3
id: unittest_existence
with:
token: ${{ secrets.GITHUB_TOKEN }}
filters: |
exists:
- added|modified: 'tests/unit/**'

- name: Check for existing comment
if: steps.unittest_existence.outputs.exists == 'false'
uses: peter-evans/find-comment@v3
id: find_comment
with:
issue-number: ${{ github.event.pull_request.number }}
comment-author: "github-actions[bot]"
body-includes: ${{ env.comment }}
- name: Comment if no unittest file was added
if: steps.unittest_existence.outputs.exists == 'false' &&
steps.find_comment.outputs.exists == 'false'
run: |
gh issue comment ${{ github.event.pull_request.number }} --repo ${{ github.repository }} --body "${{ env.comment }}"
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}

- name: Fail job if unittest is missing and required
if: steps.unittest_existence.outputs.exists == 'false'
uses: actions/github-script@v7
with:
script: core.setFailed('Unit Test required to merge.')
uses: dbt-labs/actions/.github/workflows/check-change-and-comment.yml@main
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the new action things should look much easier.
@emmyoop thoughts on this refactor and how to best test it?

with:
comment: "Thank you for your pull request! We could not find a unittest file for this change. Please consider adding some. If it is hard to unittest, please add the 'Hard to Unit Test' label. If unit tests are not needed, please add the 'Unit Test Not Needed' label."
path_filter: 'tests/unit/**'
Loading