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

New line between @expectedException and @expectedExceptionMessage #101

Open
CarsonF opened this issue Jul 28, 2017 · 6 comments
Open

New line between @expectedException and @expectedExceptionMessage #101

CarsonF opened this issue Jul 28, 2017 · 6 comments

Comments

@CarsonF
Copy link

CarsonF commented Jul 28, 2017

The Commenting\AnnotationsSniff currently doesn't like:

/**
 * @expectedException \Exception
 * @expectedExceptionMessage Don't do it.
 */
public function testThing() {}

But I don't think this looks right:

/**
 * @expectedException \Exception
 *
 * @expectedExceptionMessage Don't do it.
 */
public function testThing() {}

Could we add a blacklist for these? Possibly make it configurable?

CarsonF added a commit to bolt/codingstyle that referenced this issue Jul 28, 2017
@wickedOne
Copy link
Contributor

you could consider aliasing your annotations?

/**
 * @Tests\expectedException \Exception
 * @Tests\expectedExceptionMessage Don't do it.
 */
public function testThing() {}

should be valid.

@CarsonF
Copy link
Author

CarsonF commented Jul 29, 2017

I didn't think that worked with phpunit's annotation parser?

@wickedOne
Copy link
Contributor

i have no idea whether that's possible with phpunit's annotation parser tbh...

But I don't think this looks right

i don't think that's really the purpose of a coding standard and what you're basically asking is this coding standard to provide a way to break the rules based on sentiment.

of course feel free to create a pull request for this and try to convince @djoos of it's pupose, but personally i'm not in favour.

@djoos
Copy link
Owner

djoos commented Aug 3, 2017

Hmmm, I must admit I wasn't originally in favor either, but that is a great point @CarsonF...

@wickedOne
Copy link
Contributor

@CarsonF it looks like all your references point to test classes which clearly do not comply to a lot of standards (no class comment, no function comments, multiple classes in one file, etc.).

apart from that, if you have a look at the naming convention section of the coding standard, it's stated

Please note some early Symfony classes do not follow this convention and have not been renamed for backward compatibility reasons. However all new abstract classes must follow this naming convention;

so using the symfony source code as a reference to justify whether or not a sniff should be implemented might not be the way to go...

even though my personal opinion is that test classes should comply to coding standards as well, it's always possible to ignore those in your ci configuration

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

No branches or pull requests

3 participants