Skip to content
This repository was archived by the owner on Nov 8, 2024. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
46 changes: 46 additions & 0 deletions .github/actions/action-test/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
name: 'Test SDK'
Copy link
Contributor

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 generic

Copy link
Collaborator Author

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 with action-.

Copy link
Contributor

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

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
Expand Up @@ -4,6 +4,7 @@ on:
pull_request:
paths:
- '**/*'
workflow_dispatch:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

allows workflow to be triggered remotely via API

Copy link
Contributor

Choose a reason for hiding this comment

The 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:
Expand Down Expand Up @@ -50,17 +51,6 @@ jobs:
run-tests-with-tox:
runs-on: ubuntu-latest
steps:
- uses: "actions/checkout@v2"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

temp branch reference.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would a relative path work here and be more concise?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, yes it would, however, it requires an additional actions/checkout step to checkout the repo first, which is a step that needs to be done in the action, so it adds redundant work.

Copy link
Collaborator

Choose a reason for hiding this comment

The 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 }}
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,6 @@ __pycache__
.tox/
test-data
venv/
.DS_Store
.DS_Store
.venv
.idea
Loading