search

semsol / arc2

ARC RDF Classes for PHP
Other
332 stars 89 forks source link

Fixed store/ARC2_StoreEndpoint.php: $this has no property named $store #142

Closed ailintom closed 4 years ago

ailintom commented 4 years ago

The old version was giving an error, because $this has no property named $store

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 123

Totals Coverage Status
Change from base Build 119: 0.8%
Covered Lines: 3059
Relevant Lines: 7816
💛 - Coveralls
k00ni commented 4 years ago

Hi @ailintom, thank you. Can you please add a test to show that there was a problem before?

ailintom commented 4 years ago

Here: https://github.com/semsol/arc2/pull/143

k00ni commented 4 years ago

Hi @ailintom, thank your for #143, but it would have been better, if you had add the test in this pull request and not in a new one. I committed your test file into this pull request 7c16548, so the CI gets triggered and runs all tests again.

k00ni commented 4 years ago

Unfortunately 1 test still fails: https://travis-ci.org/github/semsol/arc2/jobs/665734128

Configuration: MariaDB 10.5 + PHP 7.4 + mysqli

Travis run: https://travis-ci.org/github/semsol/arc2/builds/665734071

Any idea?

ailintom commented 4 years ago

MySQL fails to start; hence, PHPUnit doesn't even start also. The problem is with the configuration of MySQL. It has nothing to do with the Pull Request. One option would be to increase sleep 10 in .travis/install-and-init-db.sh
What if we make it 20? People also advise specifying an ip in the docker command used to run mysql https://stackoverflow.com/questions/23234379/installing-mysql-in-docker-fails-with-error-message-cant-connect-to-local-mysq

k00ni commented 4 years ago

I increased to sleep time from 10 to 20 (#144). Lets see if Travis runs through without errors.

ailintom commented 4 years ago

Now everything seems to be fine; the only failed travis job is due to HTTP Error 429 too many requests on GitHub. I have tried to patch .travis.yml with --retry-on-http-error=429 to consider this error as a non-fatal, transient error, but wget does not recognize this command-line option

k00ni commented 4 years ago

Travis and coverage.io looking fine. Good job @ailintom