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

Setup automated macOS tests using GitHub action #2284

Merged
merged 1 commit into from Feb 14, 2024

Conversation

MattHag
Copy link
Contributor

@MattHag MattHag commented Feb 14, 2024

Related #1244

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.

@pfps
Copy link
Collaborator

pfps commented Feb 14, 2024

Is it reasonable to set up a semi-automatic build process for MacOS?

@MattHag
Copy link
Contributor Author

MattHag commented Feb 14, 2024

Yes, that is implicitly included here.

@pfps
Copy link
Collaborator

pfps commented Feb 14, 2024

Can that be put in something like a make file for MacOS?

@MattHag
Copy link
Contributor Author

MattHag commented Feb 14, 2024

Yes, both Linux and macOS commands should be moved to the Makefile in a later change. Thus, they can be reused locally.

@pfps pfps merged commit b516b12 into pwr-Solaar:master Feb 14, 2024
5 checks passed
@pfps
Copy link
Collaborator

pfps commented Feb 14, 2024

OK, I'll pull this in now.

@markopy
Copy link
Contributor

markopy commented Feb 14, 2024

@MattHag I noticed you install and start dbus via homebrew in this PR. Is that actually needed? Since we removed dbus-python Solaar isn't actually using anything dbus, right?

@MattHag
Copy link
Contributor Author

MattHag commented Feb 14, 2024

Yes it will be at least neceaary for the local setup.

@MattHag MattHag deleted the macos_test_ci_1244 branch February 14, 2024 18:55
@markopy
Copy link
Contributor

markopy commented Feb 14, 2024

What is it needed for though? dbus seems to only be used for listening to UPower events on Linux which doesn't exist on macos so I see no need for installing dbus on macos?

@MattHag
Copy link
Contributor Author

MattHag commented Feb 14, 2024

I needed it to avoid log error messages related to that on solaar startup. Maybe it's just a missing exclusion for macOS in the code.

@markopy
Copy link
Contributor

markopy commented Feb 14, 2024

I would guess those error you saw messages no longer appear now that we have excluded dbus-python from being installed on macos. The only warning you should get is failed to register suspend/resume callbacks which would be expected since there is no code to implement suspend/resume detection for macos at the moment.

@MattHag
Copy link
Contributor Author

MattHag commented Feb 14, 2024

Okay, then feel free to remove it.

@markopy
Copy link
Contributor

markopy commented Feb 14, 2024

It's your PR man. I'm just helping you out because @pfps asked for it. Nowhere in the instructions for macos does it say to install dbus so you shouldn't have added that in the first place.

MattHag added a commit to MattHag/Solaar that referenced this pull request Feb 14, 2024
pfps pushed a commit that referenced this pull request Feb 15, 2024
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

3 participants