Tests: improve performance of the sniff tests #351
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I tested this on my Ubuntu machine using the command |
The `Fixer::generateDiff()` method uses `shell_exec()` with the `diff` command to generate a file diff. Using this command is slow, in particular on Windows. This commit introduces a preliminary check in the test logic to see if a diff is even needed and skips generating the diff if no differences are expected. I don't know whether and if so, how much this will make a difference for *nix users, but on Windows, it makes a significant difference when running the sniff tests. A run of just the sniff tests without this fix takes > 1 minute (~01.03.701 last time I ran it). With this fix, the run time of the sniff tests is brought down to ~8 seconds. So let's call this a quality of life improvement for all devs which regularly need to run sniff tests for either PHPCS itself or for external standards which base their test suite on the PHPCS native test framework.
jrfnl
force-pushed
the
feature/tests-improve-performance-fixer-diff-check
branch
from
b06aa0f
to
f3eac53
Compare
|
Rebased without changes. Merging once the build passes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The
Fixer::generateDiff()method usesshell_exec()with thediffcommand to generate a file diff.Using this command is slow, in particular on Windows.
This commit introduces a preliminary check in the test logic to see if a diff is even needed and skips generating the diff if no differences are expected.
I don't know whether and if so, how much this will make a difference for *nix users, but on Windows, it makes a significant difference when running the sniff tests.
A run of just the sniff tests (
vendor/bin/phpunit tests/AllTests.php --filter Standards --no-coverage) for PHPCS without this fix takes > 1 minute (~01.03.701 last time I ran it).With this fix, the run time of the sniff tests is brought down to ~8 seconds.
So let's call this a quality of life improvement for all devs which regularly need to run sniff tests for either PHPCS itself or for external standards which base their test suite on the PHPCS native test framework.
👉🏻 Request for testing:
Based on the test runs in CI, this change should make no significant different on *nix systems, but I'd love to have some people test it and post their with and without timing results.
Suggested changelog entry
Performance improvements for the sniff tests. Should be most notable for Windows users.