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

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

Merged
merged 6 commits into from
Mar 24, 2020

Conversation

ailintom
Copy link
Contributor

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

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

coveralls commented Mar 20, 2020

Pull Request Test Coverage Report for Build 123

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.8%) to 39.138%

Totals Coverage Status
Change from base Build 119: 0.8%
Covered Lines: 3059
Relevant Lines: 7816

💛 - Coveralls

@k00ni
Copy link
Collaborator

k00ni commented Mar 21, 2020

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

@ailintom
Copy link
Contributor Author

Here: #143

Added test file `tests/unit/ARC2_StoreEndpointTest.php` for semsol#142 written by @ailintom

Now we have a test (semsol#143) which shows a problem and a potential fix (semsol#142). 
Lets see if all tests run through.

Co-authored-by: Alexander Ilin-Tomich <ilintomich@gmail.com>
@k00ni
Copy link
Collaborator

k00ni commented Mar 23, 2020

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
Copy link
Collaborator

k00ni commented Mar 23, 2020

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?

@k00ni k00ni self-assigned this Mar 23, 2020
@ailintom
Copy link
Contributor Author

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 k00ni changed the title Fix a mistake in copypasted code Fixed store/ARC2_StoreEndpoint.php: $this has no property named $store Mar 23, 2020
@k00ni
Copy link
Collaborator

k00ni commented Mar 23, 2020

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

So that wget doesn't fail on Error 429 from Github
@ailintom
Copy link
Contributor Author

ailintom commented Mar 23, 2020

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

added  travis_retry before wget to exclude error 429
@k00ni k00ni merged commit 65238a4 into semsol:master Mar 24, 2020
@k00ni
Copy link
Collaborator

k00ni commented Mar 24, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants