Skip to content

Normalize path in TypeInferenceTestCase #3449

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
Merged
merged 3 commits into from
Sep 17, 2024
Merged

Normalize path in TypeInferenceTestCase #3449

merged 3 commits into from
Sep 17, 2024

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Sep 17, 2024

since #3440 I get mixed directory separators in test-failure reporting:

There were 3 failures:

1) PHPStan\Analyser\NodeScopeResolverTest::testFile with data set "C:\dvl\Workspace\phpstan-src-staabm\tests\PHPStan\Analyser/nsrt\bug-4207.php" ('C:\dvl\Workspace\phpstan-src-...07.php')
Failed assertions in C:\dvl\Workspace\phpstan-src-staabm\tests\PHPStan\Analyser\nsrt\bug-4207.php:

...

2) PHPStan\Analyser\NodeScopeResolverTest::testFile with data set "C:\dvl\Workspace\phpstan-src-staabm\tests\PHPStan\Analyser/nsrt\bug-11692.php" ('C:\dvl\Workspace\phpstan-src-...92.php')
Failed assertions in C:\dvl\Workspace\phpstan-src-staabm\tests\PHPStan\Analyser\nsrt\bug-11692.php:

...

see the mix of \ and / in the data set names.

this PR normalizes the separator so we get now:

There were 3 failures:

1) PHPStan\Analyser\NodeScopeResolverTest::testFile with data set "C:\dvl\Workspace\phpstan-src-staabm\tests\PHPStan\Analyser\nsrt\bug-11692.php" ('C:\dvl\Workspace\phpstan-src-...92.php')
Failed assertions in C:\dvl\Workspace\phpstan-src-staabm\tests\PHPStan\Analyser\nsrt\bug-11692.php:

...

C:\dvl\Workspace\phpstan-src-staabm\tests\PHPStan\Analyser\NodeScopeResolverTest.php:257

2) PHPStan\Analyser\NodeScopeResolverTest::testFile with data set "C:\dvl\Workspace\phpstan-src-staabm\tests\PHPStan\Analyser\nsrt\range-int-range.php" ('C:\dvl\Workspace\phpstan-src-...ge.php')
Failed assertions in C:\dvl\Workspace\phpstan-src-staabm\tests\PHPStan\Analyser\nsrt\range-int-range.php:

...

@staabm
Copy link
Contributor Author

staabm commented Sep 17, 2024

//cc @schlndh

@staabm staabm marked this pull request as draft September 17, 2024 06:54
@staabm
Copy link
Contributor Author

staabm commented Sep 17, 2024

just realized this also makes another DX improvement: before this PR path were reported relative

There was 1 failure:

1) PHPStan\Analyser\NodeScopeResolverTest::testFile with data set "C:\dvl\Workspace\phpstan-src-staabm\tests\PHPStan\Analyser/../Rules/Methods/data/bug-6856.php" ('C:\dvl\Workspace\phpstan-src-...56.php')
Failed assertions in C:\dvl\Workspace\phpstan-src-staabm\tests\PHPStan\Rules\Methods\data\bug-6856.php:

after this PR these relative paths get resolved into absolute paths

There was 1 failure:

1) PHPStan\Analyser\NodeScopeResolverTest::testFile with data set "C:\dvl\Workspace\phpstan-src-staabm\tests\PHPStan\Rules\Methods\data\bug-6856.php" ('C:\dvl\Workspace\phpstan-src-...56.php')
Failed assertions in C:\dvl\Workspace\phpstan-src-staabm\tests\PHPStan\Rules\Methods\data\bug-6856.php:

@staabm staabm marked this pull request as ready for review September 17, 2024 07:02
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

mvorisek
mvorisek reviewed Sep 17, 2024
View reviewed changes
tests/PHPStan/Analyser/NodeScopeResolverTest.php
*/
public static function dataFile(): iterable
{
$base = dirname(__DIR__, 3) . '/';
Copy link
Contributor

Choose a reason for hiding this comment

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

using DIRECTORY_SEPARATOR and dirname in the findTestFiles should be enough and faster

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it would not resolve .. to real path as described in #3449 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

The 2nd part of my answer would ;-)

@ondrejmirtes
Copy link
Member

I thought you're using a Mac :) Could you also please look into the failed FileExcluderTest test on Windows on 2.0.x? Not sure what's wrong there, ideally it should work the same way it does on Unix.

@ondrejmirtes ondrejmirtes merged commit 46f343e into phpstan:1.12.x Sep 17, 2024
266 checks passed
@staabm
Copy link
Contributor Author

staabm commented Sep 17, 2024 •
edited
Loading

I thought you're using a Mac :)

I have both. windows at the primary job and a mac in my freetime.

Could you also please look into the failed FileExcluderTest test on Windows on 2.0.x?

sure

@ondrejmirtes
Copy link
Member

Thank you!

@staabm staabm deleted the norm branch September 17, 2024 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@mvorisek mvorisek mvorisek left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@staabm @phpstan-bot @ondrejmirtes @mvorisek