Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Short syntax for base assertions #384

Merged
merged 2 commits into from
Aug 1, 2015

Conversation

jubianchi
Copy link
Member

Introducing new short syntax for base assertions:

<?php

namespace tests\units;

use atoum;

class stdClass extends atoum
{
    public function testFoo()
    {
        $this
            ->variable('foo')->{'=='}('foo')
            ->variable('foo')->{'!='}('bar')

            ->object($this->newInstance)->{'=='}($this->newInstance)
            ->object($this->newInstance)->{'!='}(new \exception)
            ->object($this->newTestedInstance)->{'==='}($this->testedInstance)
            ->object($this->newTestedInstance)->{'!=='}($this->newTestedInstance)

            ->integer(42)->{42} // isEqualTo
            ->integer(42)->{'42'} // isEqualTo
            ->integer(rand(0, 10))->{'<'}(11)
            ->integer(rand(0, 10))->{'=<'}(10)
            ->integer(rand(0, 10))->{'>'}(-1)
            ->integer(rand(0, 10))->{'>='}(0)
        ;
    }
}

I was reviewing some code and remembered what we did with @mageekguy on mock call asserter to allow things like $this->mock($foo)->call('bar')->{3} to test if the bar method was called exactly three times on $foo. I asked myself if we could apply similar things to some other asserters, basically variable and integer to cover assertions that have a native operator counterpart (i.e isEqualTo, isIdenticalTo, ...).

As you have seen in the previous example, this is doable and works fine (I tested it from PHP 5.4 to 5.6). Let me know if you think it's useful ;)

@jubianchi jubianchi self-assigned this Dec 9, 2014
@jubianchi jubianchi added the POC label Dec 9, 2014
@jubianchi jubianchi force-pushed the short-assertion-syntax branch 2 times, most recently from 0d0c464 to 9e0c681 Compare December 9, 2014 23:48
@Hywan
Copy link
Member

Hywan commented Dec 10, 2014

Whist I understand the goal and I like the idea, I might not like the fact there will be four syntaxes to define an assertion:

  1. integer(42)->isEqualTo(42),
  2. integer(42)->{'=='}(42),
  3. integer(42)->{42},
  4. integer(42)->{'42'}.

The last one is very confusing because we compare an integer with a string and it would work fine…

I understand that {'=='} will be an alias to isEqualTo but I don't see a benefit while reading: It is not easier to read in my opinion. However, I could understand it would be for some people, so I am not against this PR. The only thing I request is to limit the number of syntaxes and aliases.

Finally, I don't use IDE so I don't know if it would help people or not. And what about errors also? Do you solve the alias naming?

@jubianchi
Copy link
Member Author

The syntax with string could easily be removed ;)

I agree with the fact that providing too much way of doing the same thing is not always good. I was basically playing and trying to port things I found in ruby for example. I know PHP is not Ruby and atoum is not rspec or whatever :)

BTW, I don't mind if this PR gets rejected, I played with something and that was fun :)

@mageekguy
Copy link
Contributor

It's syntaxic sugar, so, like real sugar, some which not like it don't use it.
So, i have no opinion about that (but i'm agree that using {'42'} for test equality with integer is strange and not in the atoum's way).

@devster
Copy link

devster commented Dec 10, 2014

+1

{
$assertion = null;

if (is_numeric($method) === true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

switch (true)
{
   case is_numeric($method):
      $assertion = 'isEqualTo';
      break;

   case $method == '=<':
    $assertion = 'isLessThanOrEqualTo';
        break;

   ...

@jubianchi
Copy link
Member Author

I removed the ambiguous syntax. So now, those lines won't work anymore:

<?php

namespace tests\units;

use atoum;

class stdClass extends atoum
{
    public function testFoo()
    {
        $this
            ->integer(42)->{42} // isEqualTo
            ->integer(42)->{'42'} // isEqualTo
        ;
    }
}

Why removing the two syntaxes when only the second seems invalid: because when using such syntax, PHP always casts the property/method name to a string. So we can't know if the user passed an integer or not.

@Hywan
Copy link
Member

Hywan commented Dec 10, 2014

👍 for me now :-).

@thehawk970
Copy link

👍 for me too

@jubianchi jubianchi added this to the 2.3.0 milestone Aug 1, 2015
@scrutinizer-notifier
Copy link

The inspection completed: 4 new issues, 3 updated code elements

jubianchi added a commit that referenced this pull request Aug 1, 2015
@jubianchi jubianchi merged commit abf1711 into atoum:master Aug 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants