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

[issue-558] add test for optional feature to GitHub Action #568

Merged
merged 2 commits into from
Apr 12, 2023
Merged

[issue-558] add test for optional feature to GitHub Action #568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5cc2bdc
[issue-558] add test for optional feature to GitHub Action
meretp Apr 6, 2023
2e60fdd
[issue-558] add installation group for development
meretp Apr 11, 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
5 changes: 5 additions & 0 deletions .github/workflows/install_and_test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,8 @@ jobs:
run: pytest
- name: Run CLI
run: pyspdxtools -i ./tests/spdx/data/formats/SPDXJSONExample-v2.3.spdx.json

- name: Install optional dependencies
run: python -m pip install networkx
- name: Run tests for graph generation
run: pytest tests/spdx/test_graph_generation.py
Comment on lines +37 to +40
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't we just install the optional dependencies together with the rest under "Installation"? Then we wouldn't need to call pytest again here.
By the way, is there a way to make sure that these tests actually are not skipped in the workflow?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can but I thought it would be good to test the basic installation without the optional dependencies to ensure that these tests are skipped and wouldn't cause an issue for users that don't have the optional dependencies.

Good question and good point, I will try to find out how to do this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

One approach to enforce the execution of the tests would be to set an environment variable like PYTEST_DISALLOW_SKIP and set this in the GitHub Action. Or we could check the output of pytest if the tests were executed. As all of this seems rather complicated, I think we can postpone this issue and stick to the current GitHub Action.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, let's open an issue then to keep it in the backlog.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

#572

6 changes: 3 additions & 3 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@ Here's the process to make changes to the codebase:
git checkout -b fix-or-improve-something
python -m venv ./venv
./venv/bin/activate
pip install -e .
pip install -e ".[development]"
```
Note: By using the group `[development]` for the installation, all dependencies (including optional ones) will be
installed. This way we make sure that all tests are executed.
5. Make some changes and commit them to the branch:
```sh
git commit --signoff -m 'description of my changes'
Expand All @@ -49,14 +51,12 @@ Here's the process to make changes to the codebase:
retroactively signs a range of past commits.
6. Test your changes:
```sh
pip install pytest
pytest -vvs # in the repo root
```

7. Check your code style. When opening a pull request, your changes will automatically be checked with `isort`, `black`
and `flake8` to make sure your changes fit with the rest of the code style.
```sh
pip install .[code_style]
# run the following commands in the repo root
isort src tests
black src tests
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ dynamic = ["version"]
test = ["pytest"]
code_style = ["isort", "black", "flake8"]
graph_generation = ["pygraphviz", "networkx"]
development = ["black", "flake8", "isort", "networkx", "pytest"]

[project.scripts]
pyspdxtools = "spdx.clitools.pyspdxtools:main"
Expand Down
3 changes: 1 addition & 2 deletions tests/spdx/test_graph_generation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

try:
import networkx # noqa: F401
import pygraphviz # noqa: F401
except ImportError:
pytest.skip("Skip this module as the tests need optional dependencies to run.", allow_module_level=True)

Expand Down Expand Up @@ -44,7 +43,7 @@
(
"SPDXRdfExample-v2.2.spdx.rdf.xml",
20,
17,
19,
["SPDXRef-Package_DYNAMIC_LINK", "SPDXRef-JenaLib_CONTAINS"],
),
(
Expand Down