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.

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

tests: Setup automated tests with pytest #2256

Merged
merged 2 commits into from
Feb 10, 2024
Merged

tests: Setup automated tests with pytest #2256

merged 2 commits into from
Feb 10, 2024

Conversation

MattHag
Copy link
Contributor

@MattHag MattHag commented Feb 10, 2024

Related #1097

@pfps
Copy link
Collaborator

pfps commented Feb 10, 2024

This PR just adds a testing harness, right?

The reason I ask is that a Solaar release will go out soon. If this PR doesn't affect Solaar then it can be part of the release.

@pfps
Copy link
Collaborator

pfps commented Feb 10, 2024

@MattHag
I'm not sure what failed in the tests. Can you take a look?

@MattHag
Copy link
Contributor Author

MattHag commented Feb 10, 2024

I'll take a look. Yes, it only adds support for automated tests and as side effect tests, that the package can be installed.

@pfps
Copy link
Collaborator

pfps commented Feb 10, 2024

@MattHag Does this PR also allow running the tests locally?

@MattHag
Copy link
Contributor Author

MattHag commented Feb 10, 2024

Yeah, sure.

  1. Install the package with test dependencies pip install -e .[test] to get pytest and
  2. use the command pytest to run all tests at once.

@pfps
Copy link
Collaborator

pfps commented Feb 10, 2024

@MattHag Tests work locally.

@pfps
Copy link
Collaborator

pfps commented Feb 10, 2024

@MattHag I wrote my first test - and found a bug!

@MattHag
Copy link
Contributor Author

MattHag commented Feb 10, 2024

The reason for the given error is a missing dbus dependency, that is not pre-installed in the Ubuntu base image.

I'll follow the installation page and add necessary dependencies.
https://pwr-solaar.github.io/Solaar/installation

@pfps
Copy link
Collaborator

pfps commented Feb 10, 2024

Yeah, that's a problem with installing Solaar. It depends on quite a few packages and some distributions don't have them installed by default.

@MattHag
Copy link
Contributor Author

MattHag commented Feb 10, 2024

We can (later) add a Makefile with targets for the different installations, depending on distribution and operating system. This simplifies the local setup and GitHub CI can leverage the same install procedure.

OS dependencies

For GitHub and local Ubuntu installations, you could then use the same command, e.g.
make install ubuntu
or even try to detect the OS and use a simple make install command.

Python dependencies

The pip dependencies can be installed with pip automatically. The default install would be
pip install .
and could replace all the python dependency paragraph, which requires maintenance.

@MattHag
Copy link
Contributor Author

MattHag commented Feb 10, 2024

With just two additional dependencies, it works on Ubuntu 22.04, which is used in our GitHub CI.
sudo install libdbus-1-dev libglib2.0-dev

Copy link
Collaborator

@pfps pfps left a comment

Choose a reason for hiding this comment

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

Looks good now

@pfps pfps merged commit 059ebec into pwr-Solaar:master Feb 10, 2024
3 checks passed
@MattHag MattHag deleted the setup_tests_1097 branch February 10, 2024 19:08
MattHag added a commit to MattHag/Solaar that referenced this pull request Feb 17, 2024
Replaces deprecated Node.js 16 actions.

Related pwr-Solaar#2256, pwr-Solaar#2284
pfps pushed a commit that referenced this pull request Feb 18, 2024
* Show pytest coverage in GitHub CI tests

Related #1097

* Extend Makefile with installation and test targets

Refactor setup steps to unify commands between Linux and macOS.
Move bash commands into Makefile for consistency and enable local
execution of GitHub CI commands corresponding Makefile targets.

Install on Ubuntu:
make install_ubuntu

Install on Ubuntu for development:
make install_ubuntu PIP_ARGS=."[test]"

Fixes #2303

* Improve name of GitHub test actions

Related #2303

* Upgrade GitHub actions to Node.js 20

Replaces deprecated Node.js 16 actions.

Related #2256, #2284
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants