Skip to content

Add comprehensive test suite with 100% code coverage - #133

Closed
HecFranco wants to merge 6 commits into
rectorphp:mainfrom
HecFranco:improve-test-coverage
Closed

HecFranco wants to merge 6 commits into
rectorphp:mainfrom
HecFranco:improve-test-coverage

Conversation

@HecFranco

@HecFranco HecFranco commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Expands the PHPUnit suite to 175 tests with 100% line and method coverage locally (~40% before). Adds coverage for commands, finders, node visitors, smoke test generation, and testing utilities.

All Code Analysis checks pass on PHP 8.4 (PHPUnit, PHPStan level 8, Rector, ECS, composer-dependency-analyser, etc.).

What changed

Tests

  • Command tests for all CLI commands (pretty-json, finalize-classes, privatize-constants, search-regex, smoke test generation, …)
  • Tests for FilesFinder, PhpParser node visitors, MockedClassResolver, TraitSpotter, value objects, and related services
  • Command integration fixtures (including empty-dir for pretty-json)
  • Test fixtures use local attributes instead of Doctrine (keeps composer-dependency-analyser green)

PHPUnit configuration

  • phpunit.xml — tests only (CI runs vendor/bin/phpunit with coverage: none)
  • phpunit.coverage.xml — coverage reports for local runs

Minor src/ adjustments (edge cases exposed by coverage)

  • FilesFinder::findJsonFiles() — skip empty directory lists before iterating Finder
  • Node visitors — safer handling when namespacedName is missing
  • MockWireAssert::classExists() before PHPUnit mocker creation
  • ContainerFactory — autodiscover Testing/Command
  • KernelTestCase stub — extends PHPUnit\Framework\TestCase

Local development (optional)

  • Docker (PHP 8.4): docker/test.sh, docker/coverage.sh

Coverage

Metric Before (approx.) After (local)
Lines ~40% 100%
Methods 100%
Tests ~41 175
vendor/bin/phpunit --configuration phpunit.coverage.xml

Test plan

  • vendor/bin/phpunit — 175 tests, all passing
  • composer phpstan — level 8, no errors
  • vendor/bin/composer-dependency-analyser — no issues
  • CI on PHP 8.4 — all Code Analysis jobs green
  • Coverage 100% lines/methods (local, via phpunit.coverage.xml)

Optional comment

CI is green on PHP 8.4. Coverage config is split into phpunit.coverage.xml so CI can run without pcov/xdebug. Test fixtures no longer reference Doctrine classes.

@HecFranco

Copy link
Copy Markdown
Contributor Author

Hi @TomasVotruba 👋

Thank you for reviewing and merging #131 — glad the Entropy console migration is in main now.

I've opened a follow-up PR that expands the PHPUnit suite to 100% line and method coverage (175 tests, up from ~40%). It adds tests for commands, finders, node visitors, and smoke test generation, plus a few small edge-case fixes in src/ where coverage uncovered unreachable or untested branches.

Would you have time to take a look when you get a chance? Happy to adjust anything if you'd prefer a smaller scope (e.g. tests only, without the Docker setup).

Thanks again for the merge on the previous one!

This commit introduces two new XML files, `coverage-priv.xml` and `coverage.xml`, which contain detailed coverage metrics for the `Rector\SwissKnife` project. These files provide insights into code coverage for various classes and methods, aiding in maintaining high code quality and ensuring comprehensive testing.
@TomasVotruba

TomasVotruba commented Sep 17, 2026

Copy link
Copy Markdown
Member

Thanks for your pull request!

I appreciate you taking the time to submit this; however, it appears this contribution may have been primarily AI-generated without careful human review and consideration.

We've found that AI-generated code often doesn't align well with our conventions, architectural decisions, and the specific context of what we're trying to accomplish with the framework. Quality contributions require thoughtful human insight into the codebase.

If you're interested in contributing, I'd encourage you to familiarize yourself with the existing codebase, engage with the community, and submit PRs that reflect your own understanding and careful consideration of the problem you're solving.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants