Clone test arguments

[baileylo]

If a test uses @depenends, clone the arguments that are objects to prevent unintended modification of the objects by other functions.

My biggest fear here is an bump in memory usage by phpunit, as we now duplicate multiple objects.

Fixes #1057

[whatthejeff]

This would need to be enabled somehow as it breaks backwards compatibility and not all objects are cloneable.

[whatthejeff]

Also, don't worry about the travis failure (it's related to #1052).

[baileylo]

@whatthejeff I was thinking about how to implement this so that it wouldn't be BC and would correctly handle cases where objects weren't cloneable. I was wondering if you had any feedback on these suggestions.

1. Adding an XML configuration. Since this is a global configuration it would effect all test suites, which is probably not a good idea.
2. Adding a test class docblock, eg: @dependsSideEffectSafe. I'm really bad at naming things. This solution has the same issue as my first idea but on a much smaller scale.
3. Adding/Modifying the existing @depends docblock. Either changing the current docblock to @depends FunctionName SideEffectSafe or adding @dependsSideEffectSafe FunctionName. My issue with these implementations is the programmer could still shoot themselves in the foot. See Example Below:

class Issue1075test extends PHPUnit_Framework_TestCase
{
    public function testStackInitiallyEmpty()
    {
        $stack = new SomeStack();

        $this->assertEmpty($stack->getItems());

        return $stack;
    }

    /**
     * @depends testStackInitiallyEmpty
     */
    public function testAddItem($stack)
    {
        $stack->addItem('SomeItem');

        $this->assertNotEmpty($stack->getItems());
    }

    /**
     * @depends testStackInitiallyEmpty SideEffectSafe
     */
    public function testSomethingElseThatWantsAnEmptyStack($stack)
    {
        $this->assertEmpty($stack->getItems());
    }
}

Since the object is already mutated prior to the third test running, it would clone the mutated value.

As for the case of non cloneable objects:

• If I implemented my first idea or the class wide opt-in, PHPUnit would only clone objects that could be cloned all other objects would not change how they are passed.
• If I implemented function specific opt-in, an Exception would be thrown in the case of a variable that can't be cloned.

My last thought, is this some functionality that needs to be in PHPUnit. There are ways to write tests which would prevent this issue from occurring.

[sebastianbergmann]

Have you looked at #11? IIRC, JExample (http://scg.unibe.ch/research/jexample), where the idea behind @depends comes from, offers different annotations for the different approaches.

[whatthejeff]

@sebastianbergmann–it looks like JExample uses the @Injection annotation to indicate the injection strategy. For instance:

• @Injection(InjectionPolicy.DEEPCOPY)
• @Injection(InjectionPolicy.RERUN)

[baileylo]

I started implementing multiple injection policies but ran into some issues with rerun. I'm not sure if there is a good way to rerun a "sibling" test. In the TestCase you can use result to get the top level test suite, but then you'd have to parse through the entire test suite to find the correct test. This seems very inefficient. I was thinking of injecting the TestSuite into the the TestCase, then I should be able to access sibling tests using $this->getParentTestSuite()->getTests()

My commit can be found on my local repo: baileylo/PHPUnit@b0fa94964854b09d0ce7ff557cf7f04e97805ce8.

Brief over view of what I've done:

• Added a docblock parser for @dependsInjectionPolicy POLICY
• Updated TestSuite to look for @dependsInjectionPolicy
• Implemented CLONE and NONE policies.

Issues I've run into:

• How do I handle case where object cannot be cloned and policy is CLONE?
• How should I handle re-running of a class?

[baileylo]

@whatthejeff Do you know if there is a good way to re-run tests with in a given test suite? For more context you can see my previous comment.

[sebastianbergmann]

Dear contributor,

let me start by apologizing for not commenting and/or working on the issue you have reported or merging the pull request you have sent sooner.

PHPUnit 5.0 was released today. And today I am closing all open bug reports and pull requests for PHPUnit and its dependencies that I maintain. Please do not interpret the closing of this ticket as an insult or a lack of interest in your problem. I am sorry for any inconvenience this may cause.

If the topic of this ticket is still relevant then please open a new ticket or send a new pull request. If your ticket or pull request is about a defect then please check whether the issue still exists in PHPUnit 4.8 (which will received bug fixes until August 2016). If your ticket or pull request is about a new feature then please port your patch PHPUnit 5.0 before sending a new pull request.

I hope that today's extreme backlog grooming will allow me to respond to bug reports and pull requests in a more timely manner in the future.

Thank you for your understanding,
Sebastian