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.

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

[Test] Reduce RunTestsInSeparateProcesses usage #3667

Merged
merged 10 commits into from
Apr 23, 2023
Merged

[Test] Reduce RunTestsInSeparateProcesses usage #3667

merged 10 commits into from
Apr 23, 2023

Conversation

samsonasik
Copy link
Member

@samsonasik samsonasik commented Apr 23, 2023

@staabm RunTestsInSeparateProcesses was used to cover error tests which possible due to duplicate file fixture on run tests. This was rolled back tried, rolled back, and re-tried, see https://github.com/rectorphp/rector-src/pulls?q=is%3Apr+RunTestsInSeparateProcesses+is%3Aclosed

Let's try remove reduce to see if now some that no longer needed.

@samsonasik samsonasik marked this pull request as draft April 23, 2023 09:21
@@ -2,7 +2,7 @@

namespace Rector\Tests\Renaming\Rector\FileWithoutNamespace\PseudoNamespaceToNamespaceRector\Fixture;

class Fixture6
class PhpunitTestCaseSplit
Copy link
Contributor

@staabm staabm Apr 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a new phpstan rule which makes sure classes within fixtures are uniquely named?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably needed , sometime, phpstan still read previous class "in memory", probably as "analyzed file" cache

@samsonasik samsonasik marked this pull request as ready for review April 23, 2023 09:39
@samsonasik samsonasik marked this pull request as draft April 23, 2023 09:44
@samsonasik
Copy link
Member Author

The error seems only on CI

@samsonasik
Copy link
Member Author

Let's try use FileCacheStorage for RenameClassRectorTest

@samsonasik
Copy link
Member Author

it seems using FileCacheStorage in RenameClassRector still not work, reverting.

@samsonasik
Copy link
Member Author

Let's rename the class fixture to avoid duplicate use by other rule a6f6f85

@samsonasik
Copy link
Member Author

Let's rollback RunTestsInSeparateProcesses on RenameClassRectorTest only for now. Others are removed.

@samsonasik samsonasik marked this pull request as ready for review April 23, 2023 10:49
@samsonasik samsonasik changed the title [Test] Remove RunTestsInSeparateProcesses usage [Test] Reduce RunTestsInSeparateProcesses usage Apr 23, 2023
@samsonasik
Copy link
Member Author

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

@samsonasik samsonasik merged commit 7bcda1e into main Apr 23, 2023
@samsonasik samsonasik deleted the rem-run-sep branch April 23, 2023 10:58
samsonasik added a commit that referenced this pull request May 8, 2023
* [Test] Remove RunTestsInSeparateProcesses usage

* [ci-review] Rector Rectify

* rename fixture classes

* rename fixture

* use FileCacheStorage for RenameClassRectorTest

* Revert use FileCacheStorage for RenameClassRectorTest

This reverts commit af7c37c.

* rename class

* rename fixture

* another rename

* rollback RunTestsInSeparateProcesses

---------

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants