Tests failing due to internal structure

[christian-kolb]

Describe the bug

The resulting internals of a new DateTime object differ depending on the used method of construction. This leads to failing tests depending on how the objects are constructed.

This works:

public function test_equals_works() : void
{
    $this->assertEquals(
        DateTime::fromDateTime(new \DateTimeImmutable('2022-01-01 15:00:00')),
        DateTime::fromString('2022-01-01 15:00:00')
    );
}

This doesn't:

public function test_equals_works() : void
{
    $this->assertEquals(
        DateTime::fromDateTime(new \DateTimeImmutable('2022-01-01 15:00:00')),
        new DateTime(
            new Day(
                new Month(
                    new Year(2022),
                    1
                ),
                1
            ),
            new Time(15, 0, 0),
            TimeZone::UTC()
        )
    );
}

and fails with:

Failed asserting that two objects are equal.
--- Expected
+++ Actual
@@ @@
 Aeon\Calendar\Gregorian\DateTime Object (
-    'day' => null
-    'time' => null
-    'timeZone' => null
-    'dateTime' => DateTimeImmutable Object (...)
+    'day' => Aeon\Calendar\Gregorian\Day Object (...)
+    'time' => Aeon\Calendar\Gregorian\Time Object (...)
+    'timeZone' => Aeon\Calendar\Gregorian\TimeZone Object (...)
+    'dateTime' => null
 )

Expected behavior

This should work:

public function test_equals_works() : void
{
    $this->assertEquals(
        DateTime::fromDateTime(new \DateTimeImmutable('2022-01-01 15:00:00')),
        new DateTime(
            new Day(
                new Month(
                    new Year(2022),
                    1
                ),
                1
            ),
            new Time(15, 0, 0),
            TimeZone::UTC()
        )
    );
}

Additional context

This might also impact the $clean logic within the time units like month which have the same issues.

Making the state identical for all methods on construction would also remove the side effects. For example when I construct a Time object from a string like 15:00:00 the hour, minute, ... properties won't be set. When I then call the getter hour() on it, the internal state of an "immutable" object is updated and the values are suddenly available which will might then change the test result.

I don't know whether this internal behaviour was chosen because it was the faster initial setup or whether there might be a specific reason where this solution was the only one the solve a specific issue?

Can you think of a workaround for this?

[christian-kolb]

It looks to me as the change would be as trivial as using the getter transformations in the constructor.

For example for Time instead of:

public static function fromDateTime(\DateTimeInterface $dateTime) : self
{
    if (self::$reflectionClass === null) {
        self::$reflectionClass = new \ReflectionClass(self::class);
    }

    $newTime = self::$reflectionClass->newInstanceWithoutConstructor();

    $newTime->dateTime = $dateTime instanceof \DateTime ? \DateTimeImmutable::createFromMutable($dateTime) : $dateTime;
    $newTime->clean = false;

    return $newTime;
}

it could be

public static function fromDateTime(\DateTimeInterface $dateTime) : self
{
    [$hour, $minute, $second, $microsecond] = \sscanf($dateTime->format('H-i-s.u'), '%d-%d-%d.%d');
    
    return new self($hour, $minute, $second, $microsecond);
}

This would also reduce the internal complexity and make the getters more performant.

Edit: I've just did the necessary changes for Time and created a PR for it.