DSL when checking for thrown exceptions

[dborsatto]

Hey all, this is a question (and possibily an improvement suggestion, if it makes sense) about the use of exceptions in spec tests.

Currently we do this:

$this->shouldThrow($exception)
    ->during('setSomething', [$parameter1, $parameter2]);

I believe this is a bit weird and slightly inconsistent with how regular actions are tested:

$this->getSomething()
    ->shouldBe($value);

In this, we have the action first and then an assertion, whereas with the exception syntax we have an assertion first, and then we provide the context for the assertion, which personally I find slightly counterintuitive. If I knew everything about phpspec but how to check for exceptions, I would expect the syntax to be something like this:

$this->setSomething($parameter1, $parameter2)
    ->shouldThrow($exception);

It's consistent with everything else, and it has the huge advantage of being more IDE friendly, which means refactoring a method name with PHPStorm will work with this kind of syntax, whereas what we currently have will fail because the method name is just a string. The ...->duringSetSomething($parameter1, $parameter2) syntax makes it even more difficult to find and refactor, and in my view it goes in the "too much magic" direction by obfuscating too much the actual code (though this point is absolutely a matter of personal preference).

So my question is: is there a technical reason for the choice of current syntax over the one I mention? If that's not the case, would the maintainers consider adding support for the "reverse" syntax as well? I'm not at all familiar with the internals of phpspec so I can't currently give it a go, but I might try with some help if you think it's doable.

Thank you!

[ciaranmcnulty]

There is a technical reason, although I doubt it's insurmountable. If there's a sensible implementation I'd consider it, but it's not a priority for me to work on.

You can use the shorter syntax:

$this->shouldThrow($exception)->duringSetSomething($parameter1, $parameter2);

[stof]

I don't think duringSetSomething will be detected by IDEs either. But maybe ->during()->setSomething($parameter1, $parameter2) or something like that

[dborsatto]

The only syntax that PHPStorm understands sometimes (not sure how, but 5% of the times it works) is $this->shouldThrow(...)->during('setSomething'). Using $this->shouldThrow(...)->duringSetSomething() is an absolutely no-go for me for this reason, it "hides" the real method name and also having to capitalize the first letter means I can't rely too much on full-project searches...

[stof]

Well, PHPStorm understands the current API because that's what the phpspec support in PHPStorm implements because it is the current API.
I think that my own proposal could be understood easily by PHPStorm (just like it understands magic calls for other APIs)

[ciaranmcnulty]

Whatever we do it should be consistent with other camelcase places:

shouldBeFoo
beConstructedFoo

etc

[dborsatto]

Hey there, sorry for being silent for so long. If anyone could give me some pointers (how to familiarize myself with the architecture and understand what goes on behind the scenes) I'd like to give this a shot at implementing the DSL I proposed in the original message $this->getSomething()->shouldThrow($exception), of course alongside the current implementation to avoid breaking BC 🙂