Add tests for code handling STDIN for the sniffs FileName and I18nTextDomainFixer

[rodrigoprimo]

This PR adds a test to cover code handling STDIN for the two sniffs that check it: WordPress.Files.FileName and WordPress.Utils.I18nTextDomainFixer. This is based on a related suggestion from @jrfnl for a PHPCS sniff: PHPCSStandards/PHP_CodeSniffer#681 (review).

I'm not sure if I missed something, but I could not make the ConfigDouble class work passing the standard and sniff names as parameters. I had to create a custom ruleset file and include the sniff passing its path. I believe this happens because the ConfigDouble class erases the config data, and thus, installed_paths is empty.

[jrfnl]

He @rodrigoprimo, thanks for this PR!

I'm not sure if I missed something, but I could not make the ConfigDouble class work passing the standard and sniff names as parameters. I believe this happens because the ConfigDouble class erases the config data, and thus, installed_paths is empty.

The ConfigDouble doesn't "erase" the config data, otherwise nothing would work. However, it does skip reading the CodeSniffer.conf file, which is where the installed_paths is stored.

Reason for that is threefold:

1. Performance. Reading the conf file every time a Config needs to be created in the tests has an impact on performance.
2. The user-specific CodeSniffer.conf file may also contain things like global colors or report_width overrides, which can break test presumptions.
3. Changing any settings which are be stored in the conf file on a Config in the tests will affect static variables in the Config class, which can then affect (and break) tests being run after the test making these changes.
   The ConfigDouble still allows for making those changes, but will reset the static properties on the Config class when the test has finished to prevent that.

Does that help your understanding of the ConfigDouble ?

I had to create a custom ruleset file and include the sniff passing its path.

Nice solution, though I do think there are alternative solutions which don't require the extra file.

Alternative 1: use the real Config, not the ConfigDouble.

use PHP_CodeSniffer\Config;

		$config            = new Config();
		$config->standards = ['WordPress'];
		$config->sniffs    = ['WordPress.Files.Filename'];

Not great for performance of the tests, but should work.

Alternative 2: use the PHPCSUtils 1.1.0 ConfigDouble (yes, I do still need to tag that).

use PHPCSUtils\BackCompat\Helper;
use PHPCSUtils\TestUtils\ConfigDouble;

		$config            = new ConfigDouble();
		Helper::setConfigData('installed_paths', dirname(dirname(__DIR__)), true, $config);
		$config->standards = ['WordPress'];
		$config->sniffs    = ['WordPress.Files.Filename'];

What do you think of those possibilities ?

[rodrigoprimo]

Thanks for all the information and the PR review, @jrfnl!

Alternative 1: use the real Config, not the ConfigDouble.
Not great for performance of the tests, but should work.

I can confirm that alternative 1 works indeed, as I tested it while trying to figure out why doing the same with ConfigDouble doesn't work. I discarded it due as it is mentioned in the ConfigDouble docblock that it should be used in most cases instead of Config when creating tests.

Alternative 2: use the PHPCSUtils 1.1.0 ConfigDouble (yes, I do still need to tag that).

This seems to be the best solution, and I don't see a problem with waiting until PHPCSUtils 1.1.0 is released to finish this PR. How does that sound?

[jrfnl]

Alternative 2: use the PHPCSUtils 1.1.0 ConfigDouble (yes, I do still need to tag that).

This seems to be the best solution, and I don't see a problem with waiting until PHPCSUtils 1.1.0 is released to finish this PR. How does that sound?

Works for me 👍

[jrfnl]

PR #2532 bumps PHPCSUtils to 1.1.0, so once that is merged, this PR could be updated.

[rodrigoprimo]

PR #2532 bumps PHPCSUtils to 1.1.0, so once that is merged, this PR could be updated.

Thanks for the ping, @jrfnl!

I rebased this PR and added two new commits, one for each sniff test class, updating the STDIN test to use PHPCSUtils 1.1.0 ConfigDouble class.

I did this so that each commit can be squashed into the corresponding original commit. Let me know if you prefer a different approach.

[jrfnl]

@rodrigoprimo Thanks for the update - feel free to squash those commits into the original commits already.

I've verified the tests work, despite the --sniffs directive not being respected in the tests until PHPCS 4.0 (see PHPCSStandards/PHP_CodeSniffer#996), so all good from me.

[rodrigoprimo]

Thanks for the review, @jrfnl and @dingo-d!

I just squashed the new commits into the original commits.

[jrfnl]

Thanks for the squashing @rodrigoprimo . Merging now.