Skip to content

Fix Tests#892

Merged
dahlbyk merged 11 commits intomasterfrom
fix-tests
Mar 27, 2022
Merged

Fix Tests#892
dahlbyk merged 11 commits intomasterfrom
fix-tests

Conversation

@dahlbyk
Copy link
Copy Markdown
Owner

@dahlbyk dahlbyk commented Mar 26, 2022

Following on from #878, trying to get back to all green.

  • Correctly reverts the Window Title back to original state is unreliable; might just skip
  • Expected: 'igf ' But was: 'git checkout ' seems to be a change in how either Pester or PowerShell handles -Scope Script; changing to -Scope Global seems to fix. (@dscho you were close!)
  • Tab completes add file in working dir with special char as quoted suggests core.quotepath=false (from b689b5e in Fix tab expansion for non-ASCII file paths #403) no longer affects git status -sb as expected - a path with special characters is coming back in "" regardless. This feels like a regression, but I haven't gone looking for release notes.

@dahlbyk
Copy link
Copy Markdown
Owner Author

dahlbyk commented Mar 26, 2022

  • a path with special characters is coming back in "" regardless

Turns out it's only spaces? git/git@f3fc4a1

@dahlbyk
Copy link
Copy Markdown
Owner Author

dahlbyk commented Mar 26, 2022

  • a path with special characters is coming back in "" regardless

Turns out it's only spaces? git/git@f3fc4a1

I'm genuinely confused how this test ever passed. git/git@dbfdc62 is from 2010. 🤔

@dahlbyk dahlbyk force-pushed the fix-tests branch 3 times, most recently from 5f58c7d to 153fff5 Compare March 26, 2022 20:16
Comment thread src/GitUtils.ps1
Comment thread test/DefaultPrompt.Tests.ps1 Outdated
Context 'Removing the posh-git module' {
It 'Correctly reverts the Window Title back to original state' {
Set-Item function:\prompt -Value ([Runspace]::DefaultRunspace.InitialSessionState.Commands['prompt']).Definition
$originalTitle = & $module Get-Variable OriginalWindowTitle -ValueOnly
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why not?

$originalTitle = & $module { $OriginalWindowTitle }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

But do it after importing posh-git

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Ah, now I see you're capturing this variable in the test fixture before the test runs.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Why not?

$originalTitle = & $module { $OriginalWindowTitle }

I don't use this syntax often, so I refer to

# PS> & $m Get-Variable invokeErrors -ValueOnly

Copy link
Copy Markdown
Collaborator

@rkeithhill rkeithhill left a comment

Choose a reason for hiding this comment

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

Just a few suggestions otherwise LGTM

Comment thread test/DefaultPrompt.Tests.ps1 Outdated
Context 'Removing the posh-git module' {
It 'Correctly reverts the Window Title back to original state' {
Set-Item function:\prompt -Value ([Runspace]::DefaultRunspace.InitialSessionState.Commands['prompt']).Definition
$originalTitle = & $module Get-Variable OriginalWindowTitle -ValueOnly
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

But do it after importing posh-git

Comment thread test/DefaultPrompt.Tests.ps1
Comment thread test/TabExpansion.Tests.ps1 Outdated
}
It 'Tab completes add file in working dir with special char as quoted' {
$filename = 'foo{bar} (x86).txt';
$filename = 'foo{bar} ��(x86).txt';
Copy link
Copy Markdown
Collaborator

@rkeithhill rkeithhill Mar 26, 2022

Choose a reason for hiding this comment

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

What's with the unrecognized chars? Is that part of the "special char" test? Might be worth adding as another test for emoji/unprintable chars or whatever that is. IIRC this test was meant to check for tab completion with PowerShell and/or regex special chars/syntax e.g. {} and () in the completed text.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Was just being lazy, but yeah the emoji is to ensure we're setting core.quotePath (and it's working as expected). Will break out as a separate test.

Copy link
Copy Markdown
Owner Author

@dahlbyk dahlbyk Mar 27, 2022

Choose a reason for hiding this comment

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

What's with the unrecognized chars? Is that part of the "special char" test? Might be worth adding as another test for emoji/unprintable chars or whatever that is. IIRC this test was meant to check for tab completion with PowerShell and/or regex special chars/syntax e.g. {} and () in the completed text.

Was just being lazy, but yeah the emoji is to ensure we're setting core.quotePath (and it's working as expected). Will break out as a separate test.

Turns out we already had a (better) test for this:

It 'Tab completes non-ASCII file name' {
&$gitbin config core.quotepath true # Problematic (default) config
$fileName = "posh$([char]8226)git.txt"
New-Item $fileName -ItemType File

Relocated into the appropriate Context.

Copy link
Copy Markdown
Owner Author

@dahlbyk dahlbyk left a comment

Choose a reason for hiding this comment

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

Thanks for the review, @rkeithhill

Comment thread test/DefaultPrompt.Tests.ps1 Outdated
Context 'Removing the posh-git module' {
It 'Correctly reverts the Window Title back to original state' {
Set-Item function:\prompt -Value ([Runspace]::DefaultRunspace.InitialSessionState.Commands['prompt']).Definition
$originalTitle = & $module Get-Variable OriginalWindowTitle -ValueOnly
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Why not?

$originalTitle = & $module { $OriginalWindowTitle }

I don't use this syntax often, so I refer to

# PS> & $m Get-Variable invokeErrors -ValueOnly

Comment thread test/TabExpansion.Tests.ps1 Outdated
}
It 'Tab completes add file in working dir with special char as quoted' {
$filename = 'foo{bar} (x86).txt';
$filename = 'foo{bar} ��(x86).txt';
Copy link
Copy Markdown
Owner Author

@dahlbyk dahlbyk Mar 27, 2022

Choose a reason for hiding this comment

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

What's with the unrecognized chars? Is that part of the "special char" test? Might be worth adding as another test for emoji/unprintable chars or whatever that is. IIRC this test was meant to check for tab completion with PowerShell and/or regex special chars/syntax e.g. {} and () in the completed text.

Was just being lazy, but yeah the emoji is to ensure we're setting core.quotePath (and it's working as expected). Will break out as a separate test.

Turns out we already had a (better) test for this:

It 'Tab completes non-ASCII file name' {
&$gitbin config core.quotepath true # Problematic (default) config
$fileName = "posh$([char]8226)git.txt"
New-Item $fileName -ItemType File

Relocated into the appropriate Context.

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.

2 participants