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.

Sign up for GitHub

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

Jump to bottom

Update tests.yml #83

Merged
merged 4 commits into from
Apr 16, 2021
Merged

Update tests.yml #83

merged 4 commits into from
Apr 16, 2021

Conversation

morpheus7CS
Copy link
Sponsor Contributor

Hey @calebporzio,

I've gone and updated the Github Actions test suite to reflect the support for the Laravel 7.x and 8.x versions as well as the PHP8 that @viezel has contributed recently.

From what I was able to see when testing this on a private repository, the dependency installation is no longer failing, but the build failure comes down to two test failures:

There were 2 failures:

1) Tests\SushiTest::basic_usage
Failed asserting that 2 matches expected 3.

/home/runner/work/test-sushi-ci/test-sushi-ci/tests/SushiTest.php:39

2) Tests\SushiTest::caches_sqlite_file_if_storage_cache_folder_is_available
Failed asserting that false is true.

/home/runner/work/test-sushi-ci/test-sushi-ci/tests/SushiTest.php:90

Hope this helps to get the build back to green soon.

Kind regards,
g

@morpheus7CS
Update tests.yml
344712d
@morpheus7CS
Update tests.yml
ef34672
@morpheus7CS
Update composer.json
3203456 
Changed doctrine/dbal to ^2.9, as the ^2.10 constraint excludes support for PHP 7.1
@morpheus7CS
Copy link
Sponsor Contributor Author

Hey @calebporzio,

so, I had to revert doctrine/dbalto ^2.9 constraint as ^2.10 excluded the PHP 7.1 version entirely.

Hope that now resolves most of the issues as it is purely down to the failing tests.

Kind regards,
g

@calebporzio
Copy link
Owner

Ok, I think I fixed the failing tests can you verify this update works now?

Also, do we need to composer require testbench at all?

@morpheus7CS
Copy link
Sponsor Contributor Author

Hey @calebporzio,

I pulled in the latest master branch to a separate repo and tried re-running the test there.

Feel free to check: https://github.com/morpheus7CS/sushi-ci-test/runs/1736791017?check_suite_focus=true

  • doctrine/dbal definitely has to run on ^2.9.
  • third failure will not be triggered on this repo, since it is basically triggered by the repo name change.
  • you are currently overriding the path settings in the Testbench setUp method, but this may be something PHPUnit is also capable of. 🤷‍♂️

Skipped tests trigger failure on Github Actions' side, so instead of removing them, I grouped them into a @group skippedand updated the tests.yml to run phpunit --exclude-group skipped.

The rest works and can be merged.

Thanks a lot for your work.

Kind regards,
g

@morpheus7CS morpheus7CS mentioned this pull request Jan 28, 2021
Closed
@stevebauman
Copy link

Could we get an epic merge on this bad boy? 👍

@calebporzio calebporzio merged commit 3faf827 into calebporzio:master Apr 16, 2021
@calebporzio
Copy link
Owner

Thanks so much, sorry for the ridiculous lag.

calebporzio pushed a commit that referenced this pull request Aug 11, 2021
@spawnia
Avoid error when creating database concurrently (#93)
6c7ccb9 
After introducing sushi to our project, our monitoring tool reported the following
error (stacktrace shortened for brevity):

```
PDOException: SQLSTATE[HY000]: General error: 1 table "schmelzkurve_scoring_table_results" already exists
#88 /vendor/laravel/framework/src/Illuminate/Database/Connection.php(485): PDOStatement::execute
#87 /vendor/laravel/framework/src/Illuminate/Database/Connection.php(485): Illuminate\Database\Connection::Illuminate\Database\{closure}
#86 /vendor/laravel/framework/src/Illuminate/Database/Connection.php(685): Illuminate\Database\Connection::runQueryCallback
#85 /vendor/laravel/framework/src/Illuminate/Database/Connection.php(652): Illuminate\Database\Connection::run
#84 /vendor/laravel/framework/src/Illuminate/Database/Connection.php(486): Illuminate\Database\Connection::statement
#83 /vendor/laravel/framework/src/Illuminate/Database/Schema/Blueprint.php(109): Illuminate\Database\Schema\Blueprint::build
#82 /vendor/laravel/framework/src/Illuminate/Database/Schema/Builder.php(365): Illuminate\Database\Schema\Builder::build
#81 /vendor/laravel/framework/src/Illuminate/Database/Schema/Builder.php(228): Illuminate\Database\Schema\Builder::create
#80 /vendor/calebporzio/sushi/src/Sushi.php(142): App\Models\SchmelzkurveScoringTableResult::createTable
#79 /vendor/calebporzio/sushi/src/Sushi.php(89): App\Models\SchmelzkurveScoringTableResult::migrate
#78 /vendor/calebporzio/sushi/src/Sushi.php(45): App\Models\SchmelzkurveScoringTableResult::Sushi\{closure}
#77 /vendor/calebporzio/sushi/src/Sushi.php(66): App\Models\SchmelzkurveScoringTableResult::bootSushi
```

This error can happen in rare circumstances due to a race condition.
Concurrent requests may both see the necessary preconditions for
the table creation, but only one can actually succeed.

My initial attempt at resolving this was to look for a method that
creates the table only if it does not exist (`create table if not exists`),
but Laravel exposes no such method. Thus, I think the only viable solution
is to risk it and catch the resulting error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@morpheus7CS @calebporzio @stevebauman