-
Notifications
You must be signed in to change notification settings - Fork 2
quality: parameterize branch names and extract test action #65
Changes from 4 commits
60bea96
53826c9
317e719
4775dbb
17c825c
e907e3c
e10ae06
f882436
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
name: 'Test SDK' | ||
description: 'Unit and Universal Tests' | ||
inputs: | ||
test_data_branch: | ||
type: string | ||
description: The branch in sdk-test-data to target for testcase files | ||
required: false | ||
default: main | ||
sdk_branch: | ||
type: string | ||
description: The branch of the SDK to test | ||
required: false | ||
default: main | ||
|
||
runs: | ||
using: composite | ||
steps: | ||
- name: Display Testing Details | ||
shell: bash | ||
run: | | ||
echo "Running SDK Test using" | ||
echo "Test Data: sdk-test-data@${TEST_DATA_BRANCH_NAME}" | ||
echo "SDK Branch: python-sdk@${SDK_BRANCH_NAME}" | ||
env: | ||
SDK_BRANCH_NAME: ${{ inputs.sdk_branch }} | ||
TEST_DATA_BRANCH_NAME: ${{ inputs.test_data_branch }} | ||
|
||
- uses: "actions/checkout@v2" | ||
with: | ||
repository: Eppo-exp/python-sdk | ||
ref: ${{ inputs.sdk_branch }} | ||
- uses: "actions/setup-python@v2" | ||
with: | ||
python-version: '3.9.x' | ||
- name: "Install dependencies" | ||
shell: bash | ||
run: | | ||
python -VV | ||
python -m pip install --upgrade pip setuptools wheel | ||
python -m pip install -r requirements.txt | ||
python -m pip install -r requirements-test.txt | ||
- name: "Run tests" | ||
shell: bash | ||
run: make test branchName=$BRANCH | ||
env: | ||
BRANCH: ${{ inputs.test_data_branch }} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ on: | |
pull_request: | ||
paths: | ||
- '**/*' | ||
workflow_dispatch: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. allows workflow to be triggered remotely via API There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the rename to be consistent with other SDKs as well π§Ή |
||
|
||
|
||
jobs: | ||
|
@@ -50,17 +51,6 @@ jobs: | |
run-tests-with-tox: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: "actions/checkout@v2" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. extracted into a composite action. Keeps the main workflow file cleaner in exchange for some more boilerplate and yet another yaml file |
||
- uses: "actions/setup-python@v2" | ||
with: | ||
python-version: '3.9.x' | ||
- name: "Install dependencies" | ||
run: | | ||
python -VV | ||
python -m pip install --upgrade pip setuptools wheel | ||
python -m pip install -r requirements.txt | ||
python -m pip install -r requirements-test.txt | ||
- name: 'Set up GCP SDK to download test data' | ||
uses: 'google-github-actions/setup-gcloud@v0' | ||
- name: "Run tests" | ||
run: make test | ||
- uses: Eppo-exp/python-sdk/.github/actions/action-test@tp/workflows/remote | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. temp branch reference. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would a relative path work here and be more concise? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, yes it would, however, it requires an additional There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @typotter you seem to forgot removing temp branch reference before merging? ^ This broke PR testing: https://github.com/Eppo-exp/python-sdk/actions/runs/10634064395/job/29480558050?pr=68 |
||
with: | ||
sdk_branch: ${{ github.head_ref || github.ref_name || 'main' }} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,4 +6,6 @@ __pycache__ | |
.tox/ | ||
test-data | ||
venv/ | ||
.DS_Store | ||
.DS_Store | ||
.venv | ||
.idea |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we rename this file?
action.yml
feels kind of genericThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might have read the docs wrong, but as I understand it, github expects
action.yml
for your action magic. The naming is in its containing directory where common convention is to prefix withaction-
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh right this is an action not a workflow, carry on