Introduce the constant mocker #520

Hywan · 2015-11-10T13:58:50Z

Fix #511.

In 2 steps

Split PHP mocker

The PHP mocker is splitted into 2 layers:

Abstract mocker, containing all namespace logic,
Function mocker.

The goal is to be able to introduce another mocker. So we must decouple this part first.

Introduce the constant mocker

We use it the same way we use the function mocker:

$this->constant->PHP_VERSION_ID = '606060'; // troll \o/

Note: This is used to mock constants, not class constants.

Thoughts?

jubianchi · 2015-11-10T14:02:45Z

classes/test.php

@@ -392,7 +406,8 @@ public function setAssertionManager(test\assertion\manager $assertionManager = n
 			->setHandler('stop', function() use ($test) { if ($test->debugModeIsEnabled() === true) { throw new test\exceptions\stop(); } return $test; })
 			->setHandler('executeOnFailure', function($callback) use ($test) { if ($test->debugModeIsEnabled() === true) { $test->executeOnFailure($callback); } return $test; })
 			->setHandler('dumpOnFailure', function($variable) use ($test) { if ($test->debugModeIsEnabled() === true) { $test->executeOnFailure(function() use ($variable) { var_dump($variable); }); } return $test; })
-			->setPropertyHandler('function', function() use ($test) { return $test->getPhpMocker(); })
+            ->setPropertyHandler('function', function() use ($test) { return $this->getPhpFunctionMocker(); })


indentation ;)

One morning, you will wake up, and atoum will be PSR-4…

Hywan · 2015-11-10T14:29:34Z

Tests are failing on PHP5.3 but this not seems to be related to my PR. I will fix them though.

Hywan · 2015-11-10T14:34:28Z

@Hywan No, it's your fault! Should be fixed.

jubianchi · 2015-11-10T14:34:38Z

@Hywan master is stable : https://travis-ci.org/atoum/atoum/branches

-- EDIT --
too late :D

Hywan · 2015-11-10T14:43:49Z

Green.

mageekguy · 2015-11-10T15:02:15Z

For the record, i'm not agree to merge this PR in atoum, because this new tool is an open door to bad practice about test creation and code conception.
A mock is an object, even if its behavior can be defined at runtime, so a mock should be as much as possible a black box from the test point of view.
And if you allow the developer to mock a class constant, the mock is not a black box and the developper introduce a tight coupling between its code and the test, which is a bad practice and a problem at middle term.
I'm agree that mocking a function introduce the same problem, but mock of function was introduced in atoum for a very good reason: PHP is not a full object oriented language, because its native types are not object and it has a procedural API with function and so on.
So, it can be very interesting to mock its functions because at low level, there is no other way to test the code.
However, there is another way than using a constant to know the PHP version (i know, it's just an example, but in my opinion, my point of view is valuable for all use case): just use dependency injection to mock a php version detector!
If you want to do this kind of thing (aka mock a constant), you have an architecture problem in your code and i strongly encourage you to refactor it to separate responsibility between several classes.
One of atom's goal is to help the developper to do thing right, and this tool does not help the developper to do thing right, so i can not be agree for the merge: it's definitely a -1 for me, because atoum should not provide any workaround to avoid refactoring of bad code conception.
My 2 cents.

Hywan · 2015-11-10T15:03:49Z

@mageekguy It is used to mock constants, not class constants.

mageekguy · 2015-11-10T15:05:42Z

So your PR description should be improved.
And you can forget my comment ;).

Hywan · 2015-11-10T15:06:38Z

@mageekguy ❤️

jubianchi · 2015-11-10T15:36:05Z

I'm trying to find a class layout to avoid BC break. What about:

abstract class atoum\php\mocker\builder
class atoum\php\mocker\builders\funktion extends atoum\php\mocker\builder
class atoum\php\mocker\builders\constant extends atoum\php\mocker\builder
class_alias($original = atoum\php\mocker\builders\funktion, $alias = atoum\php\mocker)

The external API will look identical, thanks to the class_alias.

Thoughts @Hywan @mageekguy ?

-- EDIT --

should be clearer like this :

<?php

namespace atoum\php\mocker {
    abstract class builder {}
}

namespace atoum\php\mocker\builders {
    use atoum\php\mocker\builder;

    class funktion extends builder {}
    class constant extends builder {}
}

namespace {
    use atoum\php\mocker;
    use atoum\php\mocker\builder;
    use atoum\php\mocker\builders\funktion;
    use atoum\php\mocker\builders\constant;

    class_alias(funktion::class, 'atoum\\php\\mocker');

    var_dump(new funktion() instanceof builder);
    var_dump(new constant() instanceof builder);

    var_dump(new funktion() instanceof mocker);
}

Hywan · 2015-11-10T15:45:06Z

@jubianchi Not the whole API will be the same. See atoum\test::setPhpMocker which become atoum\test::setPhpFunctionMocker for instance.

Hywan · 2015-11-11T09:50:32Z

And now?

Hywan · 2015-11-11T09:51:31Z

classes/test.php

@@ -282,14 +284,35 @@ public function getAdapter()

 	public function setPhpMocker(php\mocker $phpMocker = null)
 	{
-		$this->phpMocker = $phpMocker ?: new php\mocker();
+		$phpMocker = $phpMocker ?: new php\mocker();


Not sure it can work here since php\mocker does no longer exist.

indeed, it should be php\mocker\funktion

And what about:

public function setPhpMocker(php\mocker $phpMocker = null) { return $this->setPhpFunctionMocker($phpMocker); }

I guess the type-hint will warn something…

I think the polymorphism, as suggested by @mageekguy, is the best solution. It allows us to gracefully handle both mocker.

Hywan · 2015-11-13T08:43:19Z

@jubianchi So, do we need class alias?

jubianchi · 2015-11-15T22:26:15Z

@Hywan I'll try to finish this PR this week, I was busy working on the atom-plugin (which will soon be released)

Hywan · 2015-11-16T06:58:40Z

@jubianchi Excellent, thanks!

Hywan · 2015-11-24T14:59:33Z

@jubianchi Do you want me to rebase?

jubianchi · 2015-11-24T15:02:01Z

With the unit tests added, everything should be OK. But there are some limitations to correctly document.

Given the following class and test:

<?php

namespace foo {
    class foo {
        public function hello()
        {
            return PHP_VERSION_ID;
        }
    }
}

namespace tests\units\foo {
    use atoum;

    /**
     * @engine inline
     */
    class foo extends atoum
    {
        public function testFoo()
        {
            $this
                ->given($this->newTestedInstance())
                ->then
                    ->variable($this->testedInstance->hello())->isEqualTo(PHP_VERSION_ID)
                ->if($this->constant->PHP_VERSION_ID = uniqid())
                ->then
                    ->variable($this->testedInstance->hello())->isEqualTo(PHP_VERSION_ID)
            ;
        }

        public function testBar()
        {
            $this
                ->given($this->newTestedInstance())
                ->if($this->constant->PHP_VERSION_ID = $mockVersionId = uniqid()) // inline engine will fail here
                ->then
                    ->variable($this->testedInstance->hello())->isEqualTo($mockVersionId)
                ->if($this->constant->PHP_VERSION_ID = $mockVersionId = uniqid()) // isolate/concurrent engines will fail here
                ->then
                    ->variable($this->testedInstance->hello())->isEqualTo($mockVersionId)
            ;
        }
    }
}

Note the two comments: depending on the engine, the test will fail (atoum will throw exceptions) at different lines. To "mock" constant, we define them in the tested class' namespace which can only be done once.

With the inline engine this means we can only stub the constant once per test class. With isolate and concurrent engines, we can do it once per test method.

jubianchi · 2015-11-24T15:02:22Z

@Hywan yes, please do a rebase to fix merge conflicts :)

Hywan · 2015-11-24T15:03:49Z

@jubianchi I would say we have the same issue with function mocking, isn't it?

Hywan · 2015-11-24T15:06:06Z

@jubianchi Done. Maybe we can squash some commits too.

jubianchi · 2015-11-24T15:30:18Z

@Hywan go ahead ;)

Hywan · 2015-11-24T16:02:51Z

@jubianchi On PHP7 we have a segfault (https://travis-ci.org/atoum/atoum/jobs/92951761), I guess this is an error from Travis PHP build. On PHP5.3, we have a strange error, I can't take a look yet (https://travis-ci.org/atoum/atoum/jobs/92951754). If you have time, feel free to fix it, else, tell me and I will check.

jubianchi · 2015-11-25T11:18:11Z

@Hywan for the segfault I think the only workaround is to disable code coverage for some time until we figure out why it produces segfaults.

Hywan · 2015-11-25T12:41:12Z

@jubianchi I cannot reproduce. This is probably an issue with Travis' build, don't you think?

jubianchi · 2015-11-25T16:03:46Z

@Hywan if you try to reproduce it on OSX I think you won't be able to get the segfault. I never had any problem on OSX but people reported me many segfaults with xdebug on Linux (mainly Debian) and Travis envs. are built on top of it.

Hywan · 2015-11-25T20:29:03Z

@jubianchi Ok. So what's the status of this?

jubianchi · 2015-11-25T21:16:47Z

Seems good to me. One last thing I did not explain in my last comment: there is another limitation on this feature. Take a look at the testFoomethod in the previous example : the constant is used once and then stubbed. We then use the constant again and get its original value.

This is because of how PHP works and resolves names : if we mock something after it has been resolved once, we won't get the mocked value. The same applies to native function mocks.

I'll let you merge here and update the doc. issue.

Hywan · 2015-11-25T21:17:49Z

@jubianchi Yup, need to update the documentation.

The PHP mocker is splitted into 2 layers: 1. Abstract mocker, containing all namespaces logic, 2. Function mocker. The goal is to be able to introduce another mocker. So we must decouple this part first.

We use it the same way we use the function mocker: $this->constant->PHP_VERSION_ID = '606060'; // troll \o/

Introduce the constant mocker.

scrutinizer-notifier · 2015-11-25T23:11:34Z

The inspection completed: Array

Hywan added in progress enhancement labels Nov 10, 2015

Hywan assigned jubianchi Nov 10, 2015

Hywan added a commit to Hywan/atoum that referenced this pull request Nov 10, 2015

Add atoum#520.

1037646

jubianchi reviewed Nov 10, 2015
View reviewed changes

Hywan mentioned this pull request Nov 10, 2015

Document the constant mocker atoum/atoum-documentation#131

Closed

Hywan reviewed Nov 11, 2015
View reviewed changes

jubianchi added this to the 2.4.0 milestone Nov 15, 2015

Hywan force-pushed the constant_mocker branch from 2169cae to 55db167 Compare November 24, 2015 15:05

Hywan added a commit to Hywan/atoum that referenced this pull request Nov 24, 2015

Add atoum#520.

715fc51

Hywan force-pushed the constant_mocker branch from 0219a87 to 17ab828 Compare November 25, 2015 22:07

Hywan added a commit to Hywan/atoum that referenced this pull request Nov 25, 2015

Add atoum#520.

17ab828

Hywan added 3 commits November 25, 2015 23:08

Split mocker into 2 layers.

10630a1

The PHP mocker is splitted into 2 layers: 1. Abstract mocker, containing all namespaces logic, 2. Function mocker. The goal is to be able to introduce another mocker. So we must decouple this part first.

Introduce the constant mocker!

6cde45a

We use it the same way we use the function mocker: $this->constant->PHP_VERSION_ID = '606060'; // troll \o/

Add atoum#520.

6bc3c6c

Hywan force-pushed the constant_mocker branch from 17ab828 to 6bc3c6c Compare November 25, 2015 22:09

Hywan added a commit that referenced this pull request Nov 25, 2015

Merge pull request #520 from Hywan/constant_mocker

bedfb79

Introduce the constant mocker.

Hywan merged commit bedfb79 into atoum:master Nov 25, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce the constant mocker #520

Introduce the constant mocker #520

Hywan commented Nov 10, 2015

jubianchi Nov 10, 2015

Hywan Nov 10, 2015

Hywan commented Nov 10, 2015

Hywan commented Nov 10, 2015

jubianchi commented Nov 10, 2015

Hywan commented Nov 10, 2015

mageekguy commented Nov 10, 2015

Hywan commented Nov 10, 2015

mageekguy commented Nov 10, 2015

Hywan commented Nov 10, 2015

jubianchi commented Nov 10, 2015

Hywan commented Nov 10, 2015

Hywan commented Nov 11, 2015

Hywan Nov 11, 2015

jubianchi Nov 11, 2015

Hywan Nov 13, 2015

jubianchi Nov 13, 2015

Hywan commented Nov 13, 2015

jubianchi commented Nov 15, 2015

Hywan commented Nov 16, 2015

Hywan commented Nov 24, 2015

jubianchi commented Nov 24, 2015

jubianchi commented Nov 24, 2015

Hywan commented Nov 24, 2015

Hywan commented Nov 24, 2015

jubianchi commented Nov 24, 2015

Hywan commented Nov 24, 2015

jubianchi commented Nov 25, 2015

Hywan commented Nov 25, 2015

jubianchi commented Nov 25, 2015

Hywan commented Nov 25, 2015

jubianchi commented Nov 25, 2015

Hywan commented Nov 25, 2015

scrutinizer-notifier commented Nov 25, 2015

Introduce the constant mocker #520

Introduce the constant mocker #520

Conversation

Hywan commented Nov 10, 2015

Split PHP mocker

Introduce the constant mocker

jubianchi Nov 10, 2015

Choose a reason for hiding this comment

Hywan Nov 10, 2015

Choose a reason for hiding this comment

Hywan commented Nov 10, 2015

Hywan commented Nov 10, 2015

jubianchi commented Nov 10, 2015

Hywan commented Nov 10, 2015

mageekguy commented Nov 10, 2015

Hywan commented Nov 10, 2015

mageekguy commented Nov 10, 2015

Hywan commented Nov 10, 2015

jubianchi commented Nov 10, 2015

Hywan commented Nov 10, 2015

Hywan commented Nov 11, 2015

Hywan Nov 11, 2015

Choose a reason for hiding this comment

jubianchi Nov 11, 2015

Choose a reason for hiding this comment

Hywan Nov 13, 2015

Choose a reason for hiding this comment

jubianchi Nov 13, 2015

Choose a reason for hiding this comment

Hywan commented Nov 13, 2015

jubianchi commented Nov 15, 2015

Hywan commented Nov 16, 2015

Hywan commented Nov 24, 2015

jubianchi commented Nov 24, 2015

jubianchi commented Nov 24, 2015

Hywan commented Nov 24, 2015

Hywan commented Nov 24, 2015

jubianchi commented Nov 24, 2015

Hywan commented Nov 24, 2015

jubianchi commented Nov 25, 2015

Hywan commented Nov 25, 2015

jubianchi commented Nov 25, 2015

Hywan commented Nov 25, 2015

jubianchi commented Nov 25, 2015

Hywan commented Nov 25, 2015

scrutinizer-notifier commented Nov 25, 2015