[e2e] Trim low-value e2e tests, fold --only quote variants, revive orphaned rules-summary test - #8334
Merged
Merged
Conversation
…gle job Drop 3 e2e tests that only re-assert rule behaviour already covered by unit tests: * applied-rule-removed-node - fixture is byte-identical to applied-rule-removed-node-with-cache and applied-rule-removed-node-no-diffs, which both already run it * invalid-paths - covered by path validation, no engine specific behaviour * print-new-node - AddParamBasedOnParentClassMethodRector has 22 fixtures and 8 cross-file parents in its rules-test; phpstan-container-bootstrap covers the same rule end-to-end Fold the 4 "--only" quote variants into the only-option test. They shared the same config and the same expected output, and differed only in the shell quoting of a single option, yet each ran as its own CI job with 2 full composer installs. The e2e runner now accepts "--cli-options <file>", so one job runs all 5 quoting variants. E2E CI jobs: 26 -> 19.
…ry-option * applied-rule-change-docblock - RenameClassRector on a docblock is covered by applied-auto-import, RemoveUselessVarTagRector by config-dist-fallback and config-file-priority; DowngradeAttributeToAnnotationRector produced no change in its expected output at all * applied-rule-return-array-nodes - its src/ is byte-identical to only-option/src, with the same 2 rules * different-path-over-skip-config - covered by SkippedPathsResolverTest and SkipperRectorRuleTest * no-parallel-reflection-resolver - same fixture as parallel-reflection-resolver, and the non-parallel path is what every rules-test runs Found while auditing: rules-summary-option was in no workflow at all, so it never ran and its expected output had rotted to the pre "@@ Line N @@" hunk header format. Refreshed and added to the matrix, so "--rules-summary" is covered again.
This file contains hidden or 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
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.
Trims the e2e suite down to tests that cover real engine/CLI behaviour, and fixes one e2e test that silently never ran. 26 -> 16 CI jobs, each of which did 2 full
composer install.Removed: 7 tests that only re-assert unit-tested rule behaviour
applied-rule-removed-nodeapplied-rule-removed-node-with-cacheand-no-diffs, which both run itinvalid-pathsprint-new-nodeAddParamBasedOnParentClassMethodRectorhas 22 fixtures + 8 cross-fileSource/parents in its rules-test;phpstan-container-bootstrapruns the same rule end-to-endapplied-rule-change-docblockRenameClassRectoron a docblock byapplied-auto-import,RemoveUselessVarTagRectorbyconfig-dist-fallback+config-file-priority. Its third rule,DowngradeAttributeToAnnotationRector, produced no change in the expected output at allapplied-rule-return-array-nodessrc/is byte-identical toonly-option/src, with the same 2 rulesdifferent-path-over-skip-configSkippedPathsResolverTest+SkipperRectorRuleTestno-parallel-reflection-resolverparallel-reflection-resolver; the non-parallel path is what every rules-test runsFixed:
rules-summary-optionnever ranIt was present in
e2e/but listed in no workflow, so CI never executed it. Having gone unrun, its expected output had rotted to the pre-@@ Line N @@hunk header format:Refreshed and added to the matrix, so
--rules-summaryis actually covered again.Folded: 4
--onlyquote variants intoonly-optiononly-option-quote-single,-quote-single-equalnone,-quote-single-bsdoubleand-quote-double-equalnonehad identicalrector.php, identicalexpected-output.diff, and all pointed atonly-option/src. They differed only in the shell quoting of one option, yet each burned a separate CI job.The runner now takes an explicit options file, so a single job covers all 5 quotings:
All 5 quoting variants verified locally:
Kept
Everything exercising the engine or CLI that cannot be unit tested: cache reuse, parallel worker escaping / reflection / skip aggregation,
--no-diffs,--output-format=json,--rules-summary,--only,-c/--config,rector.dist.phpresolution,bootstrapFiles(), real vendor polyfill detection, and name importing.composer.jsonautoload-devdrops theno-parallel-reflection-resolverPSR-4 path;composer validatepasses.The 11 obsolete required status checks on
mainwere removed, so PRs are not blocked waiting on jobs that no longer exist.