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 sniff to check that test double/mock classes are in the correct directory #88

Merged
merged 1 commit into from
Aug 8, 2018

Conversation

jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Aug 6, 2018

This sniff looks for classes containing Mock or Double (case-insensitive) in the class name and will:

  1. Check that a tests/doubles directory exist and throw an error if it doesn't.
  2. Check that the file containing the double/mock class is within the tests/doubles directory and throw an error if it isn't.
  3. Check that the double/mock class is in a file of its own and throw an error if it isn't.

The doubles_path by default is /tests/doubles relative to the basepath for the project.
This can be customized from within a custom ruleset by passing a different value for doubles_path.

<rule ref="Yoast.Files.TestDoubles">
	<properties>
		<property name="doubles_path" value="/tests/assets"/>
	</properties>
</rule>

For this sniff to function, the basepath for the project has to be set. If it's not a warning will be thrown.
The basepath will normally be set from within the custom ruleset like so:

<arg name="basepath" value="./"/>

Includes unit tests covering all errors and warnings.

As YoastCS itself does a) not contain mocks/doubles and b) has to follow the predetermined directory structure as set by PHPCS, for the YoastCS native CS check, this sniff is disregarded (excluded).

Fixes #75


Potential changes which could be made to tighten the sniff up against false positives:

  • Limit the matching of the class name to Double / Mock at the end of the class name.
  • Limit the checking for this issue to classes found in a tests directory.

AFAICS, at this moment, no false positives are thrown without these changes and issues which previously existed and which this sniff warns about, have already been solved for the plugins (based on scans using an earlier version of this sniff).
If at some point in the future false positives would be thrown because of the above, the additional checks could be build in.

… directory

This sniff looks for classes containing `Mock` or `Double` (case-insensitive) in the class name and will:
1. Check that a `tests/doubles` directory exist and throw an `error` if it doesn't.
2. Check that the file containing the double/mock class is within the `tests/doubles` directory and throw an `error` if it isn't.
3. Check that the double/mock class is in a file of its own and throw an `error` if it isn't.

The `doubles_path` by default is `/tests/doubles` relative to the `basepath` for the project.
This can be customized from within a custom ruleset by passing a different value for `doubles_path`.
```xml
<rule ref="Yoast.Files.TestDoubles">
	<properties>
		<property name="doubles_path" value="/tests/assets"/>
	</properties>
</rule>
```

For this sniff to function, the `basepath` for the project has to be set. If it's not a `warning` will be thrown.
The basepath will normally be set from within the custom ruleset like so:
```xml
<arg name="basepath" value="./"/>
```

Includes unit tests covering all errors and warnings.

As YoastCS itself does a) not contain mocks/doubles and b) has to follow the predetermined directory structure as set by PHPCS, for the YoastCS native CS check, this sniff is disregarded (excluded).
@jrfnl jrfnl force-pushed the JRF/sniff-check-placement-testdoubles branch from 0be147b to b8227b6 Compare August 6, 2018 06:05
@moorscode
Copy link
Contributor

CR Done 👍

@dutchmartin dutchmartin merged commit f80e431 into develop Aug 8, 2018
@dutchmartin dutchmartin deleted the JRF/sniff-check-placement-testdoubles branch August 8, 2018 13:45
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.

3 participants