[Laravel 6] Bring code to a state where unit tests pass #443

bennothommo · 2019-11-22T09:13:23Z

This PR brings the state of the Library code, running Laravel 6 as the foundation, so that the unit tests pass. Whilst it is definitely no guarantee that the code will work with Laravel 6, it's at least a good start. This work forms part of the work towards octobercms/october#4381.

Changes made:

Now using Laravel 6 as the foundation framework for the library.
Now using PHPUnit 8 as opposed to 7, as Laravel 6 uses PHPUnit 8 as a dev dependency, and it will also be supported for longer. Refactored several tests to stop using deprecated assertions, which should also theoretically make them compatible with PHPUnit 9.
Now using maintainer-run Assetic library repo for bringing in Assetic library and supporting the latest Symfony Process library. Temporarily using an explicit repository definition in composer.json whilst the library is still unpublished on Packagist.
Breaking change: Changed October\Rain\Halcyon\MemoryRepository to use seconds instead of minutes, as per changes made to Illuminate\Cache\Repository. I didn't see this being used anywhere in the library, but I imagine this will likely result in needing changes done to October CMS core.
Changed some Input facade calls to comparable Request facade calls, as the Input facade is no longer available in Laravel 6 (although we are making the facade available in October still).

- Require Laravel framework components ^6.0 - Require PHPUnit 8 to take advantage of longer support timeline - Require Assetic from October CMS' Assetic repo, to fix conflict with Symfony Process

Matches Laravel 6 signature.

Prevents test from failing to load as `choice` method must be present in a class extending the Translator contract.

!! Backwards incompatible change.

@ExpectedException

Exception docblock annotations were deprecated in PHPUnit 8 and will be removed in PHPUnit 9. This changes the @ExpectedException annotation to an `expectException()` call, and the @expectedExceptionMessage annotation to an `expectExceptionMessage()` call.

The `assertInternalType()` method is deprecated in PHPUnit 8 and will be removed in PHPUnit 9. We were using them to test if arrays were returned in certain test cases, so these calls have been changed to `assertIsArray()`.

`assertContain()` calls with a string haystack is deprecated in PHPUnit 8 and will be not be supported in PHPUnit 9. This changes these calls to `assertStringContainsString()`.

`assertArraySubset()` calls are deprecated in PHPUnit 8 and will be removed in PHPUnit 9. This change brings in the "dms/phpunit-arraysubset-asserts" package which retains this assertion for PHPUnit 8 and beyond.

bennothommo · 2019-11-22T09:34:30Z

Submitted a PR to meyfa/phpunit-assert-gd to make the library compatible with PHPUnit 8, so we don't have to rewrite the Resizer unit tests. This should resolve the unit test errors reported by Actions.

meyfa/phpunit-assert-gd#2

LukeTowers · 2019-11-22T15:53:56Z

@bennothommo I've sent an email to @meyfa asking if they're interested in merging that PR. If they aren't we'll just fork it over to @octoberrain and use that instead.

src/Database/Concerns/HasRelationships.php

LukeTowers · 2019-11-22T15:58:41Z

src/Parse/Assetic/LessCompiler.php

@@ -1,13 +1,12 @@
 <?php namespace October\Rain\Parse\Assetic;

-use Event;


@bennothommo @jaxwilko @christianWilling is there value in moving this filter to the Assetic core? I'm not 100% sure what it does differently from the already included Assetic filters.

From the looks of it the October one also add's in file version hashing functionality, could merge it in this filter or add in that functionality as an option for the Assetic project?

I haven't spent enough time on this filter or the core Assetic filter to understand the difference between those two options @jaxwilko, are you able to explain it in a bit more detail for me?

@jaxwilko ping

LukeTowers · 2019-11-22T15:59:25Z

src/Parse/Assetic/ScssCompiler.php

@@ -1,12 +1,11 @@
 <?php namespace October\Rain\Parse\Assetic;

 use Event;


@bennothommo @jaxwilko @christianWilling is there value in moving this filter to the Assetic core? I'm not 100% sure what it does differently from the already included Assetic filters.

LukeTowers · 2019-11-22T16:15:57Z

@bennothommo note that the assetic package is on packagist: https://packagist.org/packages/assetic/framework#dev-2.0/dev

bennothommo · 2019-11-22T16:16:53Z

@LukeTowers sweet, will drop the repository definition in composer.json.

meyfa · 2019-11-22T20:56:11Z

@LukeTowers FYI, I've released v2.0.0 of phpunit-assert-gd with the proposed changes.

LukeTowers · 2019-11-22T23:27:54Z

@meyfa awesome, thanks very much for that! @bennothommo do you need to update the constraint in composer?

bennothommo · 2019-11-23T00:44:24Z

Thanks heaps @meyfa. Yep, will adjust composer.json accordingly, @LukeTowers :)

bennothommo · 2019-11-23T01:29:44Z

@LukeTowers Input facade has been re-added.

Newer versions deprecate, and drop, support for numeric keys in arrays, which prevents our plugin version files from working.

composer.json

Ben Thomson added 15 commits November 22, 2019 00:15

Composer dependency changes

fdbb867

- Require Laravel framework components ^6.0 - Require PHPUnit 8 to take advantage of longer support timeline - Require Assetic from October CMS' Assetic repo, to fix conflict with Symfony Process

Update test cases to use void return type where necessary

b5e6e02

Change otherKey to ownerKey for morphOne relations

12ff056

Matches Laravel 6 signature.

Fix branch alias in Composer

5a1a015

Add missing "choice" method in Translator mock.

cbd0550

Prevents test from failing to load as `choice` method must be present in a class extending the Translator contract.

Change Assetic use cases to use contracts

4f7e57d

Change MemoryRepository to use seconds instead of minutes.

a4cbe94

!! Backwards incompatible change.

Remove "nothing" test case

188ead3

Change deprecated assertInternalType() calls

2ac6691

The `assertInternalType()` method is deprecated in PHPUnit 8 and will be removed in PHPUnit 9. We were using them to test if arrays were returned in certain test cases, so these calls have been changed to `assertIsArray()`.

Change string assertContain() calls

ce9321d

`assertContain()` calls with a string haystack is deprecated in PHPUnit 8 and will be not be supported in PHPUnit 9. This changes these calls to `assertStringContainsString()`.

Require PHPUnit array subset assertion package

83139c1

`assertArraySubset()` calls are deprecated in PHPUnit 8 and will be removed in PHPUnit 9. This change brings in the "dms/phpunit-arraysubset-asserts" package which retains this assertion for PHPUnit 8 and beyond.

Fix risky tests

f23a747

Change Input facade calls to Request facade calls

8bd98f7

Fix missed exception docblock assertion calls

cca95c7

bennothommo added Status: Review Needed Type: Enhancement labels Nov 22, 2019

bennothommo requested a review from LukeTowers November 22, 2019 09:27

Testing in PHP 7.1 is now pointless

d5eb7bd

LukeTowers reviewed Nov 22, 2019

View reviewed changes

src/Database/Concerns/HasRelationships.php Outdated Show resolved Hide resolved

LukeTowers reviewed Nov 22, 2019

View reviewed changes

Revert list() method call

1d40158

Remove defined repository for assetic/framework

a859e17

Ben Thomson added 5 commits November 23, 2019 08:48

Fix documentation for input() helper

09f79a3

Use compatible meyfa/phpunit-assert-gd version

ebf54b3

Adjust Git ignores

4e88f82

Check that app is instantiated when running October facades

dcef21a

Re-add Input facade

39617a1

Ben Thomson added 4 commits November 23, 2019 21:45

Use new method for loading .env files

52cfd3c

Use correct Assetic class for ScssCompiler

83797ca

Lock symfony/yaml to version 3.3.x

f021bf3

Newer versions deprecate, and drop, support for numeric keys in arrays, which prevents our plugin version files from working.

We have to use symfony/yaml 3.4

34cd6d1

LukeTowers reviewed Nov 23, 2019

View reviewed changes

composer.json Show resolved Hide resolved

Ben Thomson added 2 commits January 18, 2020 22:13

Merge branch 'wip/laravel-5.9' into l6/unit-tests

ac0c976

Test PHP 7.4 as well

7a8e9f7

LukeTowers reviewed Jan 19, 2020

View reviewed changes

composer.json Outdated Show resolved Hide resolved

Change Laravel framework constraint back to 6.x

082989e

bennothommo merged commit bb1511a into wip/laravel-5.9 Jan 19, 2020

bennothommo deleted the l6/unit-tests branch January 19, 2020 07:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Laravel 6] Bring code to a state where unit tests pass #443

[Laravel 6] Bring code to a state where unit tests pass #443

bennothommo commented Nov 22, 2019 •

edited

Loading

bennothommo commented Nov 22, 2019

LukeTowers commented Nov 22, 2019

LukeTowers Nov 22, 2019

jaxwilko Nov 22, 2019

LukeTowers Nov 22, 2019

LukeTowers Jan 20, 2020

LukeTowers Nov 22, 2019 •

edited

Loading

LukeTowers commented Nov 22, 2019

bennothommo commented Nov 22, 2019

meyfa commented Nov 22, 2019

LukeTowers commented Nov 22, 2019

bennothommo commented Nov 23, 2019

bennothommo commented Nov 23, 2019

		@@ -1,13 +1,12 @@
		<?php namespace October\Rain\Parse\Assetic;

		use Event;

		@@ -1,12 +1,11 @@
		<?php namespace October\Rain\Parse\Assetic;

		use Event;

[Laravel 6] Bring code to a state where unit tests pass #443

[Laravel 6] Bring code to a state where unit tests pass #443

Conversation

bennothommo commented Nov 22, 2019 • edited Loading

Changes made:

bennothommo commented Nov 22, 2019

LukeTowers commented Nov 22, 2019

LukeTowers Nov 22, 2019

Choose a reason for hiding this comment

jaxwilko Nov 22, 2019

Choose a reason for hiding this comment

LukeTowers Nov 22, 2019

Choose a reason for hiding this comment

LukeTowers Jan 20, 2020

Choose a reason for hiding this comment

LukeTowers Nov 22, 2019 • edited Loading

Choose a reason for hiding this comment

LukeTowers commented Nov 22, 2019

bennothommo commented Nov 22, 2019

meyfa commented Nov 22, 2019

LukeTowers commented Nov 22, 2019

bennothommo commented Nov 23, 2019

bennothommo commented Nov 23, 2019

bennothommo commented Nov 22, 2019 •

edited

Loading

LukeTowers Nov 22, 2019 •

edited

Loading