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.

Sign up for GitHub

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

Jump to bottom

[Testing] Reduce IO on AbstractRectorTestCase take 2 #3671

Merged
merged 2 commits into from
Apr 23, 2023
Merged

[Testing] Reduce IO on AbstractRectorTestCase take 2 #3671

merged 2 commits into from
Apr 23, 2023

Conversation

samsonasik
Copy link
Member

Continue of #3670, @TomasVotruba @staabm this is reduce unnecessary FileSystem::read() when already defined in variable.

@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba I am merging it ;)

@samsonasik samsonasik merged commit 092fb22 into main Apr 23, 2023
@samsonasik samsonasik deleted the reduce-io2 branch April 23, 2023 15:56
@staabm
Copy link
Contributor

staabm commented Apr 23, 2023

Nice

@TomasVotruba
Copy link
Member

I wish we had a PHPStan rule to find these. It's seems like a obvious pattern :)

@staabm
Copy link
Contributor

staabm commented Apr 23, 2023 •
edited
Loading

I get random test error locally.

1) Rector\Tests\NodeTypeResolver\PerNodeTypeResolver\PropertyFetchTypeResolver\PropertyFetchTypeResolverTest::test with data set #7
PHPStan\File\CouldNotReadFileException: Could not read file: /Users/staabm/workspace/rector-src/rules-tests/TypeDeclaration/Rector/Property/TypedPropertyFromAssignsRector/FixtureComplexTypes/intersection_type_with_setup.php

phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/File/FileReader.php:17
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/BetterReflection/SourceLocator/FileNodesFetcher.php:29
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/BetterReflection/SourceLocator/OptimizedSingleFileSourceLocator.php:60
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/SourceLocator/Type/AggregateSourceLocator.php:26
/Users/staabm/workspace/rector-src/packages/NodeTypeResolver/Reflection/BetterReflection/SourceLocator/IntermediateSourceLocator.php:29
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/SourceLocator/Type/AggregateSourceLocator.php:26
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/SourceLocator/Type/MemoizingSourceLocator.php:33
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/Reflector/DefaultReflector.php:32
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/BetterReflection/Reflector/MemoizingReflector.php:45
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/BetterReflection/BetterReflectionProvider.php:149
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/ReflectionProvider/MemoizingReflectionProvider.php:35
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Type/ObjectType.php:1126
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Type/ObjectType.php:551
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:3327
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:3346
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:1356
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:1362
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/MutatingScope.php:558
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:1473
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:567
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:362
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:531
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:362
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:609
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:362
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:581
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Analyser/NodeScopeResolver.php:331
/Users/staabm/workspace/rector-src/packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php:374
/Users/staabm/workspace/rector-src/packages/NodeTypeResolver/PHPStan/Scope/PHPStanNodeScopeResolver.php:227
/Users/staabm/workspace/rector-src/packages/NodeTypeResolver/NodeScopeAndMetadataDecorator.php:41
/Users/staabm/workspace/rector-src/packages/Testing/TestingParser/TestingParser.php:50
/Users/staabm/workspace/rector-src/packages-tests/NodeTypeResolver/PerNodeTypeResolver/AbstractNodeTypeResolverTestCase.php:37
/Users/staabm/workspace/rector-src/packages-tests/NodeTypeResolver/PerNodeTypeResolver/PropertyFetchTypeResolver/PropertyFetchTypeResolverTest.php:35
/Users/staabm/workspace/rector-src/packages-tests/NodeTypeResolver/PerNodeTypeResolver/PropertyFetchTypeResolver/PropertyFetchTypeResolverTest.php:22

2) Rector\Tests\Naming\ValueObjectFactory\PropertyRenameFactory\PropertyRenameFactoryTest::test with data set #0
PHPStan\File\CouldNotReadFileException: Could not read file: /Users/staabm/workspace/rector-src/rules-tests/MysqlToMysqli/Rector/FuncCall/MysqlPConnectToMysqliConnectRector/Fixture/fixture.php

phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/File/FileReader.php:17
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/BetterReflection/SourceLocator/FileNodesFetcher.php:29
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/src/Reflection/BetterReflection/SourceLocator/OptimizedSingleFileSourceLocator.php:60
phar:///Users/staabm/workspace/rector-src/vendor/phpstan/phpstan/phpstan.phar/vendor/ondrejmirtes/better-reflection/src/SourceLocator/Type/AggregateSourceLocator.php:26
/Users/staabm/workspace/rector-src/packages/NodeTypeResolver/Reflection/BetterReflection/SourceLocator/IntermediateSourceLocator.php:29

I wonder why e.g. PropertyFetchTypeResolverTest reads file from TypedPropertyFromAssignsRector/FixtureComplexTypes/intersection_type_with_setup.php.

same for the 2nd failure. how/why is PropertyRenameFactoryTest dependent on MysqlPConnectToMysqliConnectRector/Fixture/fixture.php

it seems there is a hidden dependency or similar somewhere

@samsonasik
Copy link
Member Author

It probably related with how gc appplied, see

rector-src/packages/Testing/PHPUnit/AbstractRectorTestCase.php

Line 87 in 07a0475

gc_collect_cycles();

and

rector-src/tests/bootstrap.php

Line 13 in 07a0475

gc_disable();

That were there probably on purpose

samsonasik added a commit that referenced this pull request May 8, 2023
@samsonasik @actions-user
[Testing] Reduce IO on AbstractRectorTestCase take 2 (#3671)
1c37389 
* [Testing] Reduce IO on AbstractRectorTestCase take 2

* [ci-review] Rector Rectify

---------

Co-authored-by: GitHub Action <actions@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TomasVotruba TomasVotruba Awaiting requested review from TomasVotruba

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@samsonasik @staabm @TomasVotruba @actions-user