-
Notifications
You must be signed in to change notification settings - Fork 2
quality: parameterize branch names and extract test action #65
Changes from all 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: | ||
|
Collaborator
Author
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
Contributor
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" | ||
|
Collaborator
Author
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 | ||
|
Collaborator
Author
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.
Contributor
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?
Collaborator
Author
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
Collaborator
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 }} | ||
| 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.ymlfeels 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.ymlfor 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