Fix Tests #892

dahlbyk · 2022-03-26T06:17:37Z

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 · 2022-03-26T15:45:49Z

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

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

dahlbyk · 2022-03-26T19:22:37Z

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. 🤔

src/GitUtils.ps1

rkeithhill · 2022-03-26T23:15:15Z

test/DefaultPrompt.Tests.ps1

@@ -531,17 +533,12 @@ M test/Baz.Tests.ps1
        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


Why not?

$originalTitle = & $module { $OriginalWindowTitle }

But do it after importing posh-git

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

Why not?

$originalTitle = & $module { $OriginalWindowTitle }

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

posh-git/src/Utils.ps1

Line 7 in 5a87e8a

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

rkeithhill

Just a few suggestions otherwise LGTM

rkeithhill · 2022-03-26T23:16:52Z

test/DefaultPrompt.Tests.ps1

@@ -531,17 +533,12 @@ M test/Baz.Tests.ps1
        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


But do it after importing posh-git

test/DefaultPrompt.Tests.ps1

rkeithhill · 2022-03-26T23:23:57Z

test/TabExpansion.Tests.ps1

@@ -357,7 +358,7 @@ Describe 'TabExpansion Tests' {
            $result | Should -BeExactly "'v2.0.0'''"
        }
        It 'Tab completes add file in working dir with special char as quoted' {
-            $filename = 'foo{bar} (x86).txt';
+            $filename = 'foo{bar} ��(x86).txt';


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.

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:

posh-git/test/TabExpansion.Tests.ps1

Lines 222 to 226 in a62f90c

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.

dahlbyk

Thanks for the review, @rkeithhill

dahlbyk · 2022-03-27T05:14:30Z

test/DefaultPrompt.Tests.ps1

@@ -531,17 +533,12 @@ M test/Baz.Tests.ps1
        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


Why not?

$originalTitle = & $module { $OriginalWindowTitle }

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

posh-git/src/Utils.ps1

Line 7 in 5a87e8a

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

dahlbyk · 2022-03-27T05:17:27Z

test/TabExpansion.Tests.ps1

@@ -357,7 +358,7 @@ Describe 'TabExpansion Tests' {
            $result | Should -BeExactly "'v2.0.0'''"
        }
        It 'Tab completes add file in working dir with special char as quoted' {
-            $filename = 'foo{bar} (x86).txt';
+            $filename = 'foo{bar} ��(x86).txt';


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:

posh-git/test/TabExpansion.Tests.ps1

Lines 222 to 226 in a62f90c

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.

dahlbyk force-pushed the fix-tests branch 3 times, most recently from 5f58c7d to 153fff5 Compare March 26, 2022 20:16

rkeithhill reviewed Mar 26, 2022

View reviewed changes

src/GitUtils.ps1 Show resolved Hide resolved

rkeithhill reviewed Mar 26, 2022

View reviewed changes

rkeithhill approved these changes Mar 26, 2022

View reviewed changes

dahlbyk added 11 commits March 26, 2022 23:48

Format

0223178

Use module $OriginalWindowTitle

a0f71d2

Expand test alias scope

7e5297e

Skip duplicate branch runs

c3c8b1a

Extract $path1

9e4e230

Strip surrounding "" from paths with spaces

763aeca

Rearrange non-ASCII file name test

4fbf49e

Test Windows PowerShell, too

4b7bc51

Show Pester version

abf1781

Cancel in-progress

425b7e1

Avoid Remove-Alias

77c0f42

dahlbyk commented Mar 27, 2022

View reviewed changes

dahlbyk force-pushed the fix-tests branch from 81d1609 to 77c0f42 Compare March 27, 2022 05:18

dahlbyk merged commit ec83c77 into master Mar 27, 2022

dahlbyk deleted the fix-tests branch March 27, 2022 05:24

dahlbyk mentioned this pull request Mar 31, 2022

Modifying tab completion for files to be relative. #626

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Tests #892

Fix Tests #892

dahlbyk commented Mar 26, 2022 •

edited

Loading

dahlbyk commented Mar 26, 2022

dahlbyk commented Mar 26, 2022

rkeithhill Mar 26, 2022

rkeithhill Mar 26, 2022

rkeithhill Mar 26, 2022

dahlbyk Mar 27, 2022

rkeithhill left a comment

rkeithhill Mar 26, 2022

rkeithhill Mar 26, 2022 •

edited

Loading

dahlbyk Mar 27, 2022

dahlbyk Mar 27, 2022 •

edited

Loading

dahlbyk left a comment

dahlbyk Mar 27, 2022

dahlbyk Mar 27, 2022 •

edited

Loading

	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

Fix Tests #892

Fix Tests #892

Conversation

dahlbyk commented Mar 26, 2022 • edited Loading

dahlbyk commented Mar 26, 2022

dahlbyk commented Mar 26, 2022

rkeithhill Mar 26, 2022

Choose a reason for hiding this comment

rkeithhill Mar 26, 2022

Choose a reason for hiding this comment

rkeithhill Mar 26, 2022

Choose a reason for hiding this comment

dahlbyk Mar 27, 2022

Choose a reason for hiding this comment

rkeithhill left a comment

Choose a reason for hiding this comment

rkeithhill Mar 26, 2022

Choose a reason for hiding this comment

rkeithhill Mar 26, 2022 • edited Loading

Choose a reason for hiding this comment

dahlbyk Mar 27, 2022

Choose a reason for hiding this comment

dahlbyk Mar 27, 2022 • edited Loading

Choose a reason for hiding this comment

dahlbyk left a comment

Choose a reason for hiding this comment

dahlbyk Mar 27, 2022

Choose a reason for hiding this comment

dahlbyk Mar 27, 2022 • edited Loading

Choose a reason for hiding this comment

dahlbyk commented Mar 26, 2022 •

edited

Loading

rkeithhill Mar 26, 2022 •

edited

Loading

dahlbyk Mar 27, 2022 •

edited

Loading

dahlbyk Mar 27, 2022 •

edited

Loading