Use GitHub Actions for testing #330
Conversation
|
Latest testrun can be seen here: https://github.com/marc1706/web-push-php/runs/2705838241?check_suite_focus=true |
tests/PushServiceTest.php
Outdated
| @@ -42,7 +42,7 @@ public static function setUpBeforeClass(): void | |||
| */ | |||
| protected function setUp(): void | |||
| { | |||
| if (!(getenv('TRAVIS') || getenv('CI'))) { | |||
| if (getenv('TRAVIS') || getenv('CI')) { | |||
There was a problem hiding this comment.
Can we remove that altogether? The description below is wrong, this should be run on CI (these are the tests that really matter actually)
There was a problem hiding this comment.
The description is wrong, yes. But the test itself has some issues running, e.g.
https://github.com/marc1706/web-push-php/runs/2808589708?check_suite_focus=true
There was a problem hiding this comment.
Yes, that's why the tests are "broken" right now, and this is what needs to be fixed
There was a problem hiding this comment.
To clarify, the tests are correct, this is a problem with web-push-testing-service
There was a problem hiding this comment.
web-push-testing-service seems to be no longer maintained and has been archived: https://github.com/GoogleChromeLabs/web-push-testing-service
I've also tested around with it a bit more and even after updating it to newer dependencies it's inherently unstable and fails very often. Unless there is another way of having some form of functional testing I would actually recommend to remove these.
PHP 8.1 is currently not supported by this.
|
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. 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. 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 |
|
@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. 👍 |
|
@Minishlink Addressed your comments. Updated "build" branch (this PR + commit to run tests) tests have ran through: |
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