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

Jump to bottom

ci: port lint, unit test, and e2e tests to Actions #155

Merged
merged 15 commits into from
Apr 19, 2023
Merged

ci: port lint, unit test, and e2e tests to Actions #155

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
5f7c890
Add unit test + lint test on GA
ankatiyar Apr 3, 2023
8ce58f6
trigger GA - will revert
ankatiyar Apr 3, 2023
1167fc4
Fix lint
ankatiyar Apr 3, 2023
4ad9683
Add end to end tests
ankatiyar Apr 6, 2023
f6a723f
Add cache key
ankatiyar Apr 11, 2023
6f8e75e
Add cache action
ankatiyar Apr 12, 2023
bd90e45
Rename workflow files
ankatiyar Apr 12, 2023
1bd6a94
Lint + add comment + default bash
ankatiyar Apr 12, 2023
e41d8bd
Merge branch 'main' into github-actions-unit-tests
ankatiyar Apr 18, 2023
f47cb2c
Add windows test
ankatiyar Apr 18, 2023
6770ff9
Update workflow name + revert changes to READMEs
ankatiyar Apr 19, 2023
6552c0d
Add kedro-telemetry/RELEASE.md to trufflehog ignore
ankatiyar Apr 19, 2023
8203daa
Add pytables to test_requirements remove from workflow
ankatiyar Apr 19, 2023
545912f
Revert "Add pytables to test_requirements remove from workflow"
ankatiyar Apr 19, 2023
2e79e69
Separate pip freeze step
ankatiyar Apr 19, 2023
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
16 changes: 16 additions & 0 deletions .github/workflows/airflow-ci.yml
View file
Open in desktop
ankatiyar marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
name: Run tests on kedro-airflow
ankatiyar marked this conversation as resolved.
Show resolved Hide resolved

on:
push:
paths:
- "kedro-airflow/**"
pull_request:
paths:
- "kedro-airflow/**"
types: [ synchronize ]

jobs:
airflow-test:
uses: ./.github/workflows/unit-test.yml
with:
plugin: kedro-airflow
16 changes: 16 additions & 0 deletions .github/workflows/datasets-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
name: Run tests on kedro-datasets

on:
push:
paths:
- "kedro-datasets/**"
pull_request:
paths:
- "kedro-datasets/**"
types: [ synchronize ]

jobs:
datasets-test:
uses: ./.github/workflows/unit-test.yml
with:
plugin: kedro-datasets
15 changes: 15 additions & 0 deletions .github/workflows/docker-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
name: Run tests on kedro-docker

on:
push:
paths:
- "kedro-docker/**"
pull_request:
paths:
- "kedro-docker/**"
types: [ synchronize ]
jobs:
ankatiyar marked this conversation as resolved.
Show resolved Hide resolved
docker-test:
uses: ./.github/workflows/unit-test.yml
with:
plugin: kedro-docker
16 changes: 16 additions & 0 deletions .github/workflows/telemetry-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
name: Run tests on kedro-telemetry

on:
push:
paths:
- "kedro-telemetry/**"
pull_request:
paths:
- "kedro-telemetry/**"
types: [ synchronize ]

jobs:
telemetry-test:
uses: ./.github/workflows/unit-test.yml
with:
plugin: kedro-telemetry
79 changes: 79 additions & 0 deletions .github/workflows/unit-test.yml
View file
Open in desktop
ankatiyar marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
name: Unit and linting tests

on:
workflow_call:
inputs:
plugin:
type: string
jobs:
unit-tests:
strategy:
matrix:
os: [ubuntu-latest]
python-version: ["3.7", "3.8", "3.9", "3.10"]
runs-on: ${{ matrix.os }}
steps:
- name: Checkout code
uses: actions/checkout@v3
- name: Set up Python ${{matrix.python-version}}
uses: actions/setup-python@v3
with:
python-version: ${{matrix.python-version}}
cache: 'pip'
ankatiyar marked this conversation as resolved.
Show resolved Hide resolved
- name: Install dependencies
run: |
cd ${{ inputs.plugin }}
ankatiyar marked this conversation as resolved.
Show resolved Hide resolved
pip install git+https://github.com/kedro-org/kedro@main
pip install -r test_requirements.txt
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pip install git+https://github.com/kedro-org/kedro@main
pip install -r test_requirements.txt
pip install git+https://github.com/kedro-org/kedro@main -r test_requirements.txt

Didn't check if this is also there is the existing CI, but we should not have multiple pip install commands, because they can override dependencies installed in a previous step.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current CI setup does it this way as well, kedro is installed first and then the test_requirements.txt for the plugin being tested. I tried making this change in my forked repo but the tests start failing at the "Installing dependencies" stage for kedro-datasets because of dependency resolution conflict. (See this failed run)

Copy link
Member

Choose a reason for hiding this comment

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

I think this is a critical problem that needs resolving--whether it's in scope of this PR or not is a separate issue.

What this means is that, in reality, we don't have resolvable requirements, and we're only able to get to a resolvable state by overwriting some of the previously-installed requirements. Some of the stuff installed in the pip install -r test_requirements.txt are not actually going to be compatible with Kedro on main, it seems.

Copy link
Member

Choose a reason for hiding this comment

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

This is definitely something we need to look into more. I'd suggest creating a separate ticket.

pip freeze
- name: Run unit tests
run: make plugin=${{ inputs.plugin }} test

lint:
runs-on: ubuntu-latest
steps:
- name: Checkout code
uses: actions/checkout@v3
- name: Set up Python 3.8
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Any reason not to lint on the latest supported version instead of 3.8?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Current setup also runs lint on 3.8. The lint tests fail for other versions of python for kedro-datasets because of this snowflake-snowpark-python is only installed for python version 3.8 so pylint throws import errors. We can change the python version to 3.10 and suppress the error

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for answering, I'm fine with either route (or doing this in a separate task in the future); I was just wondering. :)

uses: actions/setup-python@v3
with:
python-version: 3.8
cache: 'pip'
- name: Install dependencies
run: |
cd ${{ inputs.plugin }}
pip install git+https://github.com/kedro-org/kedro@main
pip install -r test_requirements.txt
Copy link
Member

Choose a reason for hiding this comment

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

Same

Suggested change
pip install git+https://github.com/kedro-org/kedro@main
pip install -r test_requirements.txt
pip install git+https://github.com/kedro-org/kedro@main -r test_requirements.txt

pip freeze
- name: Install pre-commit hooks
run: |
cd ${{ inputs.plugin }}
pre-commit install --install-hooks
pre-commit install --hook-type pre-push
- name: Run linter
run: make plugin=${{ inputs.plugin }} lint

e2e-tests:
if: inputs.plugin != 'kedro-datasets'
strategy:
matrix:
os: [ ubuntu-latest ]
python-version: [ "3.7", "3.8", "3.9", "3.10" ]
runs-on: ${{ matrix.os }}
steps:
- name: Checkout code
uses: actions/checkout@v3
- name: Set up Python ${{matrix.python-version}}
uses: actions/setup-python@v3
with:
python-version: ${{matrix.python-version}}
cache: 'pip'
- name: Install dependencies
run: |
cd ${{ inputs.plugin }}
pip install git+https://github.com/kedro-org/kedro@main
pip install -r test_requirements.txt
pip freeze
- name: Run end to end tests
shell: 'script -q -e -c "bash {0}"'
ankatiyar marked this conversation as resolved.
Show resolved Hide resolved
run: make plugin=${{ inputs.plugin }} e2e-tests
2 changes: 1 addition & 1 deletion kedro-airflow/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Kedro-Airflow

[will revert - trigger GA]
[![License](https://img.shields.io/badge/license-Apache%202.0-blue.svg)](https://opensource.org/licenses/Apache-2.0)
[![Python Version](https://img.shields.io/badge/python-3.7%20%7C%203.8%20%7C%203.9%20%7C%203.10-blue.svg)](https://pypi.org/project/kedro-airflow/)
[![PyPI Version](https://badge.fury.io/py/kedro-airflow.svg)](https://pypi.org/project/kedro-airflow/)
Expand Down
2 changes: 1 addition & 1 deletion kedro-datasets/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Kedro-Datasets

[will revert - trigger GA]
[![License](https://img.shields.io/badge/license-Apache%202.0-blue.svg)](https://opensource.org/licenses/Apache-2.0)
[![Python Version](https://img.shields.io/badge/python-3.7%20%7C%203.8%20%7C%203.9%20%7C%203.10-blue.svg)](https://pypi.org/project/kedro-datasets/)
[![PyPI Version](https://badge.fury.io/py/kedro-datasets.svg)](https://pypi.org/project/kedro-datasets/)
Expand Down
2 changes: 1 addition & 1 deletion kedro-docker/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Kedro-Docker

[will revert - trigger GA]
[![Python Version](https://img.shields.io/badge/python-3.7%20%7C%203.8%20%7C%203.9%20%7C%203.10-blue.svg)](https://pypi.org/project/kedro-docker/)
[![PyPI version](https://badge.fury.io/py/kedro-docker.svg)](https://pypi.org/project/kedro-docker/)
[![License](https://img.shields.io/badge/license-Apache%202.0-blue.svg)](https://opensource.org/licenses/Apache-2.0)
Expand Down
2 changes: 1 addition & 1 deletion kedro-telemetry/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Kedro-Telemetry

[will revert - trigger GA]
[![Python Version](https://img.shields.io/badge/python-3.7%20%7C%203.8%20%7C%203.9%20%7C%203.10-blue.svg)](https://pypi.org/project/kedro-telemetry/)
[![PyPI version](https://badge.fury.io/py/kedro-telemetry.svg)](https://pypi.org/project/kedro-telemetry/)
[![License](https://img.shields.io/badge/license-Apache%202.0-blue.svg)](https://opensource.org/licenses/Apache-2.0)
Expand Down
1 change: 1 addition & 0 deletions trufflehog-ignore.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
kedro-telemetry/README.md
kedro-datasets/tests/tensorflow/test_tensorflow_model_dataset.py