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

(ci) test with lowest dependency required version #170

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

Sysix
Copy link

@Sysix Sysix commented Feb 13, 2024

At the moment we install all our dependencies with the highest minor & patch number.
Because this project support all major versions from the beginning (X.0) we should also test these specific versions too.

There is a composer command which will help us: --prefer-lowest
From the composer docs it suggest to also include --prefer-stable

@luisdalmolin
Copy link
Member

This makes sense, thanks @Sysix. But the tests are failing. I think it's because of the PHPUnit dependency, but not quite sure.

@Sysix
Copy link
Author

Sysix commented Feb 15, 2024

PHP 8 Support is only available for phpunit 9.3:
https://github.com/sebastianbergmann/phpunit/milestone/38?closed=1

Also testbench support laravel 8 at version 6.*:
https://packages.tools/testbench.html

EDIT: when its still not working, we need to drop the phpunit 9 support. Should not be that much of a problem 🤞

@Sysix
Copy link
Author

Sysix commented Feb 20, 2024

PHPUnit 10 supports only PHP8.1. PHPUnit 9.5 removed the deprecation warning

Then another wierd Bug was happening with testbench < 6.24:

There was 1 failure:

  1. Kirschbaum\PowerJoins\Tests\JoinRelationshipTest::test_has_one_of_many
    Failed asserting that 'select "posts".*, "comments"."body", max("votes") as "votes", "comments"."post_id" from "posts" inner join "comments" on "comments"."post_id" = "posts"."id" where "posts"."deleted_at" is null limit 1' contains "max("comments"."votes") as "votes_aggregate"".

/var/www/html/tests/JoinRelationshipTest.php:765

When requiring testbench 6.24 this bug is fixed. Don't know why :/

EDIT: I looked which version is tested for laravel. The tests require laravel 8.75.0 so we should also reflect that.

@Sysix
Copy link
Author

Sysix commented Feb 21, 2024

the CI overwrote our requirements. Updated the github workflow too :)

@luisdalmolin
Copy link
Member

Still erroring. I'm not opposed on dropping support for outdated Laravel/PHP versions, if that helps

@Sysix
Copy link
Author

Sysix commented Mar 8, 2024

I forgot one line to update.

@danharrin danharrin force-pushed the master branch 2 times, most recently from e1e576a to d858f29 Compare October 2, 2024 20:03
@danharrin
Copy link
Contributor

Hey @Sysix, we updated this based on the latest master, but it appears to be broken as the testbench versions are now not being passed through to that installation command. It would be great if you had any ideas about what is wrong!

.github/workflows/ci.yaml Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants