web-push-libs / web-push-php

Web Push library for PHP
MIT License
1.69k stars 295 forks source link

Use GitHub Actions for testing #330

Closed marc1706 closed 2 years ago

marc1706 commented 3 years ago

With this PR, tests will be run on GitHub Actions instead of Travis CI. Previously disabled tests will work again.

On a side note, the pure PHP implementation without GMP does not work and causes the library to not work on PHP 7.2 and earlier if GMP is missing. In general, I think it would make sense to remove these and the gmp requirement and fully rely on PHP's integrated openssl functions. That would however have to be done in a separate issue / pull request.

Should also resolve #274

marc1706 commented 3 years ago

Latest testrun can be seen here: https://github.com/marc1706/web-push-php/runs/2705838241?check_suite_focus=true

marc1706 commented 2 years ago

Well, I spent my Christmas trying to get the web-push-testing-service to run. Then I tried directly interacting with browsers via chromedriver & geckodriver by using php-webdriver. That also failed, partially due to bad support for the push API in those drivers and also because even tests that work locally often times fail on the CI due to them being very resource intensive. Push testing with the geckodriver is for example currently completely broken: https://github.com/mozilla/geckodriver/issues/1687

After spending countless hours on this, I finally decided to do what we often times during testing: Implement a testing replacement for what we want to test. I have therefore created this tool to test subscription, sending notifications, and retrieving their content: https://github.com/marc1706/web-push-testing It allows subscribing in the same way one would subscribe in a browser but does also support sending notifications to an endpoint and then retrieving them to check if all of this worked. One major upside being that it's super quick and has none of the issues I have encountered with web-push-testing-service or using php-webdriver. In addition to that, I also spent a bit of time on implementing a good test coverage to ensure that it's doing what it's supposed to be doing. Is it the same as a browser? Of course not. Does it need to be a browser in CI? In my opinion that's a clear no. Test the API, not the overhead of trying to use an API in a complete browser.

Anyhow, enough of a rant. Builds currently pass on the branch with the same commits plus one extra commit to ensure that it's being built: https://github.com/marc1706/web-push-php/runs/4732073431?check_suite_focus=true

marc1706 commented 2 years ago

@Minishlink I was wondering if you could maybe have another look at this PR. To summarize my post above, I have basically replaced the testing against a browser with testing against a representation of the API used by browsers. Looking forward to your feedback. 👍

marc1706 commented 2 years ago

@Minishlink Addressed your comments. Updated "build" branch (this PR + commit to run tests) tests have ran through: https://github.com/marc1706/web-push-php/actions/runs/1798861831