Problem with promises when cloning doubles

[elvetemedve]

When a Double object is cloned, the attached promises won't be cloned. Therefore the two objects are stitched together.

The example below demonstrates the problem.

<?php

require_once 'vendor/autoload.php';

use Prophecy\Argument;

// Initialize Prophecy
$prophet = new Prophecy\Prophet;
$prophecy = $prophet->prophesize();

// Setup mock
$prophecy->willExtend('DateTime');
$prophecy->setTimestamp(Argument::any())->will(function($arguments) {
    $this->getTimestamp()->willReturn($arguments[0]);
});

$dummyDateTime = $prophecy->reveal();

// Use the doubles within the test subject

$dummyDateTimeClone = clone $dummyDateTime;

$dummyDateTime->setTimestamp(123);
$dummyDateTimeClone->setTimestamp(456);

echo 'original double: ';
var_dump(get_class($dummyDateTime));

echo 'cloned double: ';
var_dump(get_class($dummyDateTimeClone));

echo 'call getTimestamp() on original = ';
var_dump($dummyDateTime->getTimestamp());

echo 'call getTimestamp() on cloned = ';
var_dump($dummyDateTimeClone->getTimestamp());

Actual Output

original double: /home/gbuza/Downloads/test/test.php:27:
string(18) "Double\DateTime\P1"
cloned double: /home/gbuza/Downloads/test/test.php:30:
string(18) "Double\DateTime\P1"
call getTimestamp() on original = /home/gbuza/Downloads/test/test.php:33:
int(456)
call getTimestamp() on cloned = /home/gbuza/Downloads/test/test.php:36:
int(456)

Expected Output

original double: /home/gbuza/Downloads/test/test.php:27:
string(18) "Double\DateTime\P1"
cloned double: /home/gbuza/Downloads/test/test.php:30:
string(18) "Double\DateTime\P1"
call getTimestamp() on original = /home/gbuza/Downloads/test/test.php:33:
int(123)
call getTimestamp() on cloned = /home/gbuza/Downloads/test/test.php:36:
int(456)

Environment

Prophecy version: 1.11.1
PHP version: 7.4.9

[DonCallisto]

What's the point of cloning a double?

[ciaranmcnulty]

I guess the SUT does the doubling?

[DonCallisto]

@ciaranmcnulty if it is as you guess, is pretty much useless to clone the double as the internal one (the SUT one) would be a totally different instance.

[stof]

Well, as DateTime is a value object, your example would be much cleaner by not using a double at all, but using an actual DateTime object.

The issue you have here is that cloning the double still creates a double linked to the same ObjectProphecy, and so they don't have independent states.

[elvetemedve]

I guess the SUT does the doubling?

@ciaranmcnulty Yes, that's the case.

Well, as DateTime is a value object, your example would be much cleaner by not using a double at all, but using an actual DateTime object.

@stof That's true, but I picked DateTime as an example. I run into this problem on a project where the SUT is not a value object.

The issue you have here is that cloning the double still creates a double linked to the same ObjectProphecy, and so they don't have independent states.

That's happening exactly. Unfortunately deep cloning is not supported by PHP out of the box, so I can't fix it that way. Introducing a factory method could be a workaround.

[stof]

@elvetemedve if we were cloning the ObjectProphecy when cloning the method, this might solve some things, but it might introduce other issues.

[DonCallisto]

I think that cloning the ObjectProphecy (no matter if shallow or deep clone) has no value.
If it is cloned in SUT code, what is executed inside the SUT is a code that hits a totally different double from test one.
So, what's the point here? We already can't make assumption (stub methods, expectations) in test.
Am I missing something?

[stof]

Looking at it more in details, it seems very hard to implement __clone in a way cloning the double while maintaining consistent state of the double and its associated ObjectProphecy and LazyDouble (inside the ObjectProphecy) due to being a circular object graph.
And expectations cannot work either as the Prophet class won't know about the cloned prophecy.

@ciaranmcnulty should we generate a __clone method in doubles throwing an exception saying that cloning a Prophecy double is not supported, for easier debugging of why things don't work ?

[DonCallisto]

And expectations cannot work either as the Prophet class won't know about the cloned prophecy.

Exactly. So I'm in favor of this

we generate a __clone method in doubles throwing an exception saying that cloning a Prophecy double is not supported, for easier debugging of why things don't work