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 CI #192

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Setup CI #192

wants to merge 9 commits into from

Conversation

sgrodzicki
Copy link
Contributor

As mentioned in #169 (comment) this is the foundation for setting up unit tests (to be extended with integration tests).

Probably the most controversial is the bump to PHP 5.6, which is due to the supported versions of the testing framework. On the other hand, PHP 5 has not been supported for a long time, so this is, in my opinion, a good time to move at least a little bit closer to the current supported versions of PHP.

@malle-pietje
Copy link
Collaborator

Thanks for the PR!

I fully agree wrt PHP versions; the only reason we have kept that requirement low is because there are still developers using a variety of PHP 5.X versions. Here are the latest stats from Packagist:
image

I'll dive into this asap.

@sgrodzicki
Copy link
Contributor Author

It's 0% for all of 5.x recently, so perhaps we could use PHPUnit 8 and include PHP 7&8 instead of PHP 5&7?

I'm OK with leaving things as is (>=5.6) as this would enforce that no new syntax (e.g., strict_types) is used while only keeping us from running automated tests against PHP 8. However, if you're willing to drop PHP 5 altogether I'm happy to upgrade all dependencies.

@malle-pietje
Copy link
Collaborator

I think we should have PHP 7 as the minimum requirement. We may need to add a note to the readme explaining how users can still use the class with PHP 5 (for all install methods mentioned).

@sgrodzicki
Copy link
Contributor Author

I think we should have PHP 7 as the minimum requirement. We may need to add a note to the readme explaining how users can still use the class with PHP 5 (for all install methods mentioned).

Great! Upgraded to PHPUnit 8 (and as a consequence to PHP 7.2) in d54d18a

@sgrodzicki
Copy link
Contributor Author

I figured that we can still allow PHP 5 and only run tests for supported PHP versions (>=7.2). Changes in 2a8c353.

@sgrodzicki
Copy link
Contributor Author

I've added integration tests in eefcb65 which test against every minor version of PHP starting from 7.2 and all UniFi Network Application versions available as Docker images. This currently translates into the following matrix:

PHP / UniFi stable-5 stable-6 v7.0 v7.1 v7.2 v7.3
7.2 PHP 7.2.34 / UniFi 5.14.23 PHP 7.2.34 / UniFi 6.5.55 PHP 7.2.34 / UniFi 7.0.25 PHP 7.2.34 / UniFi 7.1.68 PHP 7.2.34 / UniFi 7.2.95 PHP 7.2.34 / UniFi 7.3.83
7.3 PHP 7.3.33 / UniFi 5.14.23 PHP 7.3.33 / UniFi 6.5.55 PHP 7.3.33 / UniFi 7.0.25 PHP 7.3.33 / UniFi 7.1.68 PHP 7.3.33 / UniFi 7.2.95 PHP 7.3.33 / UniFi 7.3.83
7.4 PHP 7.4.33 / UniFi 5.14.23 PHP 7.4.33 / UniFi 6.5.55 PHP 7.4.33 / UniFi 7.0.25 PHP 7.4.33 / UniFi 7.1.68 PHP 7.4.33 / UniFi 7.2.95 PHP 7.4.33 / UniFi 7.3.83
8.0 PHP 8.0.28 / UniFi 5.14.23 PHP 8.0.28 / UniFi 6.5.55 PHP 8.0.28 / UniFi 7.0.25 PHP 8.0.28 / UniFi 7.1.68 PHP 8.0.28 / UniFi 7.2.95 PHP 8.0.28 / UniFi 7.3.83
8.1 PHP 8.1.17 / UniFi 5.14.23 PHP 8.1.17 / UniFi 6.5.55 PHP 8.1.17 / UniFi 7.0.25 PHP 8.1.17 / UniFi 7.1.68 PHP 8.1.17 / UniFi 7.2.95 PHP 8.1.17 / UniFi 7.3.83
8.2 PHP 8.2.4 / UniFi 5.14.23 PHP 8.2.4 / UniFi 6.5.55 PHP 8.2.4 / UniFi 7.0.25 PHP 8.2.4 / UniFi 7.1.68 PHP 8.2.4 / UniFi 7.2.95 PHP 8.2.4 / UniFi 7.3.83

@malle-pietje
Copy link
Collaborator

@sgrodzicki It would be good to revisit this topic again. I haven't looked into this in great detail but what would the general flow be? Also, how do we determine which controller and site the tests are executed a against for the assertions?

@sgrodzicki
Copy link
Contributor Author

The test matrix is defined in .github/workflows/test.yml and it looks like v7.4, v7.5, and v8.0 would need to be added. Also, 8.3 for PHP itself. Other than that everything seems to be working fine. Feel free to have a look at the output of those test runs: https://github.com/sgrodzicki/UniFi-API-client/actions/runs/7247262033

@malle-pietje
Copy link
Collaborator

Thanks. Looking at the results and the setup of the tests it looks as if you're assuming the localhost is running MongoDB and we have an account with given username and password. Could we obtain the credentials and URL from a file that is outside of source control? Then restrict the tests to the login and other methods.

Forgive the ignorant questions; I'm not that familiar yet with PHP testing 😉

@sgrodzicki
Copy link
Contributor Author

This setup is meant to run on GitHub Actions but it could certainly be adjusted to run somewhere else (though Docker would be almost identical). Those credentials are defaults from the core components.

@malle-pietje
Copy link
Collaborator

Okay, understood. Will dive into this next year 😉

@malle-pietje
Copy link
Collaborator

FYI, we've bumped PHP requirement to 7.4 😉

@sgrodzicki
Copy link
Contributor Author

FYI, we've bumped PHP requirement to 7.4 😉

Upgraded to PHP 7.4 and updated dependencies (GitHub Actions & UniFi versions)

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.

2 participants