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

Run tests against fondant package with tox #283

Merged
merged 7 commits into from
Jul 10, 2023
Merged

Run tests against fondant package with tox #283

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
789c555
Add tox for testing
RobbeSneyders Jul 10, 2023
ec11f60
Move fondant package into src directory
RobbeSneyders Jul 10, 2023
beaacf7
Update imports with fondant as third party package
RobbeSneyders Jul 10, 2023
a279508
Run tox with poetry in github action
RobbeSneyders Jul 10, 2023
8f06a0a
Install using poetry in check-licenses tox environment
RobbeSneyders Jul 10, 2023
b730746
Add tox-gh-actions test dependency
RobbeSneyders Jul 10, 2023
56b40dc
Add soft version constraints in tox.ini
RobbeSneyders Jul 10, 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
19 changes: 3 additions & 16 deletions .github/workflows/pipeline.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,9 @@ jobs:
run: |
pip install --upgrade pip
pip install poetry==1.4.0
poetry install --all-extras
- name: Test with pre-commit
run: poetry run pre-commit run --all-files --show-diff-on-failure
- name: Check licenses
run: |
poetry export -f requirements.txt --output /tmp/requirements.txt
poetry run liccheck -s license_strategy.ini -r /tmp/requirements.txt -l PARANOID
- name: Shadow-run pylint
# Check if pylint catches errors not caught by our pre-commit checks to validate switch
# to ruff
run: |
pip install pylint==2.16.4
pylint fondant --exit-zero
- name: Test with pytest
run: |
poetry run pytest tests --cov fondant --cov-report term-missing
poetry install --all-extras --with test
- name: Test with tox
run: tox
- name: Coveralls
run: |
pip install "coveralls<4"
Expand Down
4 changes: 1 addition & 3 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,8 @@ pandas = { version = ">= 1.3.5", optional = true }
pipelines = ["kfp", "kubernetes"]

[tool.poetry.group.test.dependencies]
liccheck = "^0.7.3"
pre-commit = "^3.1.1"
pytest = "^7.2.2"
pytest-cov = "^4.1.0"
tox = "^4.6.4"

[tool.poetry.group.docs.dependencies]
mkdocs-material = "^9.1.8"
Expand Down
2 changes: 1 addition & 1 deletion scripts/pre-build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ scripts_path=$( cd "$(dirname "${BASH_SOURCE[0]}")" ; pwd -P )
root_path=$(dirname "$scripts_path")

pushd "$root_path"
rm fondant/components
rm -rf fondant/components
cp -r components/ fondant/
popd
39 changes: 39 additions & 0 deletions tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
[tox]
isolated_build = True
envlist =
pre-commit
Copy link
Contributor

Choose a reason for hiding this comment

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

do these dependencies only get built when running tox then? does it automatically invoke pip install on them since we no longer have them in the pyproject.toml

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, installs in this file are only installed by tox when executing the related environment. If a dependency is only needed for a single environment type, I moved them from pyproject.toml to here, and I added tox to pyproject.toml.

Copy link
Contributor

Choose a reason for hiding this comment

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

alright thanks for the clarification

check-licenses
py{38,39,310,311}

[gh-actions]
python =
3.8: py38
3.9: py39
3.10: pre-commit,check-licenses, py310
3.11: py311

[testenv:pre-commit]
deps=pre-commit
commands=pre-commit run --all-files --show-diff-on-failure

[testenv:check-licenses]
deps=
liccheck==0.7.3
Copy link
Contributor

Choose a reason for hiding this comment

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

can we have soft dependency versions here like we had in pyproject.toml? (same for pytest, and pytest-cov)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. Updated.

commands=
poetry export -f requirements.txt --output /tmp/requirements.txt
poetry run liccheck -s license_strategy.ini -r /tmp/requirements.txt -l PARANOID

[testenv]
setenv=PYTHONPATH = {toxinidir}:{toxinidir}
deps=
pytest==7.4.0
pytest-cov==4.1.0
allowlist_externals=
poetry
/usr/bin/bash
commands=
bash ./scripts/pre-build.sh
poetry lock
poetry install --all-extras
poetry show
poetry run python -m pytest tests --cov fondant --cov-report term-missing