Allow Carbon 2.0

[ruudk]

This library is locked to Carbon 1.22. It contains an annoying JsonSerializable interface that conflicts with my editor. This has been fixed in 2.1 (briannesbitt/Carbon@20d3f33#diff-c958cc2b645be4b72d33e0d6dd23c070)

[antonioribeiro]

Tests on Carbon 2 are not passing, that's maybe why Laravel is still locked on 1.26+, right?

Configuration: /Users/antoniocarlosribeiro/code/pragmarx/ia-collection/phpunit.xml

  Warning - The configuration file did not pass validation!
  The following problems have been detected:

  Line 14:
  - Element 'phpunit', attribute 'syntaxCheck': The attribute 'syntaxCheck' is not allowed.

  Test results may not be as expected.


..................................EE...F.......................  63 / 290 ( 21%)
............................................................... 126 / 290 ( 43%)
............................................................... 189 / 290 ( 65%)
............................................................... 252 / 290 ( 86%)
......................................                          290 / 290 (100%)

Time: 1.63 seconds, Memory: 14.00MB

There were 2 errors:

1) IlluminateAgnostic\Collection\Tests\Support\SupportCarbonTest::testCarbonIsMacroableWhenNotCalledStatically
BadMethodCallException: Method IlluminateAgnostic\Collection\Support\Carbon::addYears does not exist.

/Users/antoniocarlosribeiro/code/pragmarx/ia-collection/src/Support/Traits/Macroable.php:99
/Users/antoniocarlosribeiro/code/pragmarx/ia-collection/tests/Support/SupportCarbonTest.php:47

2) IlluminateAgnostic\Collection\Tests\Support\SupportCarbonTest::testCarbonIsMacroableWhenCalledStatically
BadMethodCallException: Method IlluminateAgnostic\Collection\Support\Carbon::subDays does not exist.

/Users/antoniocarlosribeiro/code/pragmarx/ia-collection/src/Support/Traits/Macroable.php:99
/Users/antoniocarlosribeiro/code/pragmarx/ia-collection/tests/Support/SupportCarbonTest.php:53
/Users/antoniocarlosribeiro/code/pragmarx/ia-collection/src/Support/Traits/Macroable.php:81
/Users/antoniocarlosribeiro/code/pragmarx/ia-collection/tests/Support/SupportCarbonTest.php:56

--

There was 1 failure:

1) IlluminateAgnostic\Collection\Tests\Support\SupportCarbonTest::testCarbonCanSerializeToJson
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
 Array &0 (
+    'localMonthsOverflow' => null
+    'localYearsOverflow' => null
+    'localStrictModeEnabled' => null
+    'localHumanDiffOptions' => null
+    'localToStringFormat' => null
+    'localSerializer' => null
+    'localMacros' => null
+    'localGenericMacros' => null
+    'localTranslator' => null
+    'dumpLocale' => null
     'date' => '2017-06-27 13:14:15.000000'
     'timezone_type' => 3
     'timezone' => 'UTC'
 )

/Users/antoniocarlosribeiro/code/pragmarx/ia-collection/tests/Support/SupportCarbonTest.php:94

ERRORS!
Tests: 290, Assertions: 807, Errors: 2, Failures: 1.

[antonioribeiro]

On Laravel 5.7 we still have a failing test:

PHPUnit 7.3.5 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.1.20 with Xdebug 2.5.1
Configuration: /Users/antoniocarlosribeiro/code/pragmarx/ia-collection/phpunit.xml

........................................F......................  63 / 296 ( 21%)
............................................................... 126 / 296 ( 42%)
............................................................... 189 / 296 ( 63%)
............................................................... 252 / 296 ( 85%)
............................................                    296 / 296 (100%)

Time: 1.16 seconds, Memory: 14.00MB

There was 1 failure:

1) IlluminateAgnostic\Collection\Tests\Support\SupportCarbonTest::testCarbonCanSerializeToJson
Failed asserting that '2017-06-27T13:14:15.000000Z' is identical to Array &0 (
    'date' => '2017-06-27 13:14:15.000000'
    'timezone_type' => 3
    'timezone' => 'UTC'
).

/Users/antoniocarlosribeiro/code/pragmarx/ia-collection/tests/Support/SupportCarbonTest.php:94

FAILURES!
Tests: 296, Assertions: 827, Failures: 1.

Generating code coverage report in Clover XML format ... done

Generating code coverage report in HTML format ... done


Code Coverage Report:
  2018-09-20 18:33:19

 Summary:
  Classes: 25.00% (2/8)
  Methods: 85.57% (172/201)
  Lines:   89.02% (689/774)

[ruudk]

I think Carbon fixed the serialization, so that it always serializes to ISO8106?

[ruudk]

Yep, https://github.com/briannesbitt/Carbon/blob/44f2b0586e7c953a66746bda6a9ac487414fcf77/tests/Carbon/JsonSerializationTest.php#L68

The test in this library should be changed :)

[antonioribeiro]

Just sent a PR to add that very same fix to 1.33+

briannesbitt/Carbon#1440

[antonioribeiro]

Yep, https://github.com/briannesbitt/Carbon/blob/44f2b0586e7c953a66746bda6a9ac487414fcf77/tests/Carbon/JsonSerializationTest.php#L68

The test in this library should be changed :)

Yeah, but only if Laravel moves to 2.1+, because in 1.33 serialization is still returning an array, which is odd...

[ruudk]

Thanks

What if the test allows either array or string? Then it can support both 1.x and 2.x?

[kylekatarnls]

Hi, Carbon 2 now gives a string as JSON, but it can be configured, see: https://carbon.nesbot.com/docs/#api-carbon-2

It's a major version, it has major changes. But for Carbon 2, they a should not be a big deal to adapt to, just follow this part of the documentation.

Laravel is not "locked", only 5.6 has 1.26 for stability reason, Laravel 5.7 allows 1.x and Laravel 5.8 should allows Carbon 2. We have an open pull-request for that and have to take the right decision about how to integrate it properly, and also allow CarbonImmutable easily in Laravel.

So I agree @ruudk, allow 1 and 2 using json settings to get the same output or simply allow both would be wise.

[antonioribeiro]

My hands are kind of tied here, because this package is Laravel Collections extracted, it is supposed to be the exact same you get on Laravel. Extraction is even automated by https://github.com/antonioribeiro/ia-collection/blob/master/upgrade.sh, so unless they fix/upgrade Carbon stuff, there's nothing I can do.

[kylekatarnls]

This will pass if this is merged: laravel/framework#25320

[antonioribeiro]

@ruudk PR was merged by @kylekatarnls (thank you!), but you'll still have to wait for Carbon 1.34 to be tagged.

[ruudk]

Thanks both! I'll wait for the tag :)

[kylekatarnls]

1.34 is released.