fix: Resolve localized strings from the shipped file on every host

CHANGELOG.md

@@ -77,6 +77,30 @@
 
 ### Fixed
 
+- [**#185**](https://github.com/psake/PowerShellBuild/issues/185)
+  Consumers on a non-English Windows PowerShell 5.1 host get the module's
+  real messages. `PowerShellBuild.psm1` carried a hand-maintained second copy
+  of every string, bound whenever `Import-LocalizedData` resolved nothing, and
+  it had drifted sixteen strings behind `en-US/Messages.psd1` — every
+  certificate and signing message was absent, and the Pester floor was still
+  reported as `5.0.0`. That lookup resolves by UI culture and the module ships
+  `en-US` only: PowerShell 7 falls back to `en-US` and never saw the drift,
+  but Windows PowerShell 5.1 does not fall back at all, so a French or
+  Japanese install bound the stale copy. A missing string does not throw, so
+  the symptom was a blank `WARNING:` line, or a bare `ScriptHalted` where the
+  signing tasks meant to say no certificate was found. The copy is gone,
+  `en-US` is now requested by name when the culture lookup misses, and an
+  import that cannot find the strings at all fails outright rather than
+  blanking every message.
+
+  The same sweep found one string the module read and never shipped:
+  `Publish-PSBuildModule` validated `-Path` against `PathDoesNotExist`, which
+  `en-US/Messages.psd1` did not define, so passing a path that does not exist
+  failed with a blank message on **every** host and culture while the very
+  next check in the same validation block reported itself properly. The string
+  is now defined, and a test asserts that every string the module reads is one
+  it ships.
+
 - [**#124**](https://github.com/psake/PowerShellBuild/issues/124)
   The docs tree can hold documentation that is not generated help. A `README.md`
   at its root, a `CONTRIBUTING.md` beside the generated markdown, an `images/` or
@@ -133,7 +157,7 @@
   that passed before may now correctly fail.
 - [**#96**](https://github.com/psake/PowerShellBuild/issues/96)
   `Test-PSBuildScriptAnalysis` no longer fails with a path-resolution error
   when `SettingsPath` is not supplied. An unsupplied path was forwarded to
   PSScriptAnalyzer as `-Settings ''`, which resolved against the current
   directory and threw before any analysis ran, so the function's own
   documented example could not run as written.

PowerShellBuild/PowerShellBuild.psm1

@@ -9,42 +9,25 @@ foreach ($import in $public + $private) {
     }
 }
 
-data LocalizedData {
-    # Load here in case Import-LocalizedData is not available
-    ConvertFrom-StringData @'
-NoCommandsExported=No commands have been exported. Skipping markdown generation.
-FailedToGenerateMarkdownHelp=Failed to generate markdown help. : {0}
-AddingFileToPsm1=Adding [{0}] to PSM1
-MakeCabNotAvailable=MakeCab.exe is not available. Cannot create help cab.
-DirectoryAlreadyExists=Directory already exists [{0}].
-PathLongerThan3Chars=Path [{0}] must be longer than 3 characters.
-BuildSystemDetails=Build System Details:
-BuildModule=Build Module:       {0}:{1}
-PowerShellVersion=PowerShell Version: {0}
-EnvironmentVariables={0}Environment variables:
-PublishingVersionToRepository=Publishing version [{0}] to repository [{1}]...
-FolderDoesNotExist=Folder does not exist: {0}
-PathArgumentMustBeAFolder=The Path argument must be a folder. File paths are not allowed.
-UnableToFindModuleManifest=Unable to find module manifest [{0}]. Can't import module
-PesterTestsFailed=One or more Pester tests failed
-PesterVersionNotSupported=Pester version [{0}] is loaded, but Test-PSBuildPester requires Pester 5.0.0 or newer.
-CodeCoverage=Code Coverage
-Type=Type
-CodeCoverageLessThanThreshold=Code coverage: [{0}] is [{1:p}], which is less than the threshold of [{2:p}]
-CodeCoverageCodeCoverageFileNotFound=Code coverage file [{0}] not found.
-SeverityThresholdSetTo=SeverityThreshold set to: {0}
-PSScriptAnalyzerResults=PSScriptAnalyzer results:
-ScriptAnalyzerErrors=One or more ScriptAnalyzer errors were found!
-ScriptAnalyzerWarnings=One or more ScriptAnalyzer warnings were found!
-ScriptAnalyzerIssues=One or more ScriptAnalyzer issues were found!
-'@
-}
-$importLocalizedDataSplat = @{
+# Every user-facing string lives in en-US/Messages.psd1 and nowhere else. Import-LocalizedData
+# resolves that file through the current UI culture and the culture's parent chain, so the lookup
+# misses whenever the machine's UI culture has no directory of its own -- and this module ships
+# en-US only. PowerShell 7 has a final en-US fallback that hides the miss; Windows PowerShell 5.1
+# has none and binds nothing at all, which is why a French or Japanese Windows install got no
+# strings. Ask for en-US by name when the culture lookup comes up empty, and fail the import if
+# even that is not there: silent, missing strings turn every message into an empty warning or a
+# bare ScriptHalted (psake/PowerShellBuild#185).
+$importLocalizedDataParameters = @{
     BindingVariable = 'LocalizedData'
     FileName        = 'Messages.psd1'
     ErrorAction     = 'SilentlyContinue'
 }
-Import-LocalizedData @importLocalizedDataSplat
+Import-LocalizedData @importLocalizedDataParameters
+if (-not $LocalizedData) {
+    $importLocalizedDataParameters['UICulture'] = 'en-US'
+    $importLocalizedDataParameters['ErrorAction'] = 'Stop'
+    Import-LocalizedData @importLocalizedDataParameters
+}
 
 
 Export-ModuleMember -Function $public.Basename

PowerShellBuild/en-US/Messages.psd1

@@ -1,4 +1,4 @@
-ConvertFrom-StringData @'
+ConvertFrom-StringData @'
 NoCommandsExported=No commands have been exported. Skipping markdown generation.
 FailedToGenerateMarkdownHelp=Failed to generate markdown help. : {0}
 AddingFileToPsm1=Adding [{0}] to PSM1
@@ -13,6 +13,7 @@ PowerShellVersion=PowerShell Version: {0}
 EnvironmentVariables={0}Environment variables:
 PublishingVersionToRepository=Publishing version [{0}] to repository [{1}]...
 FolderDoesNotExist=Folder does not exist: {0}
+PathDoesNotExist=Path does not exist: {0}
 PathArgumentMustBeAFolder=The Path argument must be a folder. File paths are not allowed.
 UnableToFindModuleManifest=Unable to find module manifest [{0}]. Can't import module
 PesterTestsFailed=One or more Pester tests failed

tests/LocalizedData.tests.ps1

@@ -0,0 +1,195 @@
+# Coverage for how the module resolves its user-facing strings at import
+# (psake/PowerShellBuild#185).
+#
+# PowerShellBuild.psm1 used to carry a second, hand-written copy of every string in an inline
+# `data LocalizedData { ConvertFrom-StringData ... }` block, kept for the case where
+# Import-LocalizedData resolved nothing. The import runs with -ErrorAction SilentlyContinue, so a
+# lookup that misses binds that copy without saying anything -- and the copy went sixteen strings
+# stale, still naming the pre-#182 Pester floor of 5.0.0.
+#
+# The lookup misses more often than it looks. Import-LocalizedData resolves Messages.psd1 through
+# the current UI culture and that culture's parent chain, and the module ships en-US only.
+# PowerShell 7 has a final en-US fallback, so it always lands on the shipped file; Windows
+# PowerShell 5.1 has no such fallback, so on a machine whose display language is French or
+# Japanese the lookup binds nothing at all and the stale copy is what consumers got. 5.1 is a
+# supported host (`PowerShellVersion = '5.1'`), which is what made the drift reachable rather
+# than theoretical.
+#
+# Two things are pinned here. The first It is the drift guard: one definition of the strings, so
+# there is no second copy left to fall out of step. The Context below is the behavioral
+# guarantee, and it runs in a child process per supported host -- a UI culture has to be in place
+# before the module is imported, and setting it in-process would leak into the rest of the suite.
+
+BeforeDiscovery {
+    # Probe the host running the suite, plus Windows PowerShell when it is installed, because the
+    # two do not agree about the en-US fallback and only the 5.1 leg catches the regression. The
+    # current process is used for the first leg rather than a PATH lookup so this always produces
+    # at least one leg, whichever host the suite was started with; identical paths collapse.
+    $currentHostPath = [System.Diagnostics.Process]::GetCurrentProcess().Path
+    $windowsPowerShellPath = (
+        Get-Command -Name 'powershell.exe' -CommandType 'Application' -ErrorAction SilentlyContinue |
+            Select-Object -First 1
+    ).Source
+
+    # Sort-Object -Unique rather than Select-Object -Unique: when the suite is running under
+    # Windows PowerShell the two lookups return the same executable spelled differently
+    # (powershell.EXE and powershell.exe), and only Sort-Object collapses that by default.
+    $script:localizationHost = @(
+        $currentHostPath, $windowsPowerShellPath |
+            Where-Object { $_ } |
+            Sort-Object -Unique |
+            ForEach-Object { @{ HostName = [IO.Path]::GetFileName($_); HostPath = $_ } }
+    )
+    if ($script:localizationHost.Count -eq 0) {
+        # An empty -ForEach collection throws during discovery, which reads as a file that
+        # silently disappeared. Say what actually went wrong instead.
+        throw 'Could not resolve the path of the host running this suite; no host can be probed.'
+    }
+}
+
+Describe 'Localized string resolution' {
+
+    BeforeAll {
+        $script:repositoryRoot = Split-Path -Path $PSScriptRoot -Parent
+        $script:sourceModulePath = [IO.Path]::Combine(
+            $script:repositoryRoot, 'PowerShellBuild', 'PowerShellBuild.psm1'
+        )
+
+        # Probe the built module rather than the source tree: it is what a consumer installs, and
+        # the versioned directory is the -BaseDirectory Import-LocalizedData needs.
+        $sourceManifestPath = [IO.Path]::Combine(
+            $script:repositoryRoot, 'PowerShellBuild', 'PowerShellBuild.psd1'
+        )
+        $moduleVersion = (Import-PowerShellDataFile -Path $sourceManifestPath).ModuleVersion
+        $script:builtModulePath = [IO.Path]::Combine(
+            $script:repositoryRoot, 'Output', 'PowerShellBuild', $moduleVersion
+        )
+        # The probe imports the .psm1 directly, not the manifest. The manifest's RequiredModules
+        # are resolved against the child process's inherited PSModulePath, which on Windows
+        # PowerShell means the PowerShell 7 module directories it was spawned from -- a failure
+        # that has nothing to do with which strings got bound. The .psm1 is the file under test.
+        $script:builtModuleFilePath = Join-Path -Path $script:builtModulePath -ChildPath 'PowerShellBuild.psm1'
+
+        # The strings as shipped. Read through Import-LocalizedData because Messages.psd1 is a
+        # ConvertFrom-StringData document rather than a hashtable literal, so
+        # Import-PowerShellDataFile cannot read it.
+        $importShippedStringParameters = @{
+            BindingVariable = 'shippedString'
+            BaseDirectory   = $script:builtModulePath
+            FileName        = 'Messages.psd1'
+            UICulture       = 'en-US'
+            ErrorAction     = 'Stop'
+        }
+        Import-LocalizedData @importShippedStringParameters
+        $script:shippedString = $shippedString
+
+        # The probe reports what the module bound at import, from inside the module's own scope.
+        # Written to $TestDrive rather than checked in, following the convention in
+        # Test-PSBuildPester.tests.ps1 for generated fixtures.
+        $script:probeScriptPath = Join-Path -Path $TestDrive -ChildPath 'Get-ResolvedString.ps1'
+        Set-Content -Path $script:probeScriptPath -Value @'
+param(
+    [Parameter(Mandatory)]
+    [string]
+    $ModuleFilePath,
+
+    [Parameter(Mandatory)]
+    [string]
+    $UICulture
+)
+
+# Set before the import, because Import-LocalizedData reads the UI culture as the module loads.
+# This is the state a machine whose Windows display language is not English starts up in.
+[System.Threading.Thread]::CurrentThread.CurrentUICulture =
+    [System.Globalization.CultureInfo]::new($UICulture)
+
+Import-Module -Name $ModuleFilePath -Force -WarningAction SilentlyContinue -ErrorAction Stop
+$resolvedString = & (Get-Module -Name 'PowerShellBuild') { $LocalizedData }
+$resolvedString | ConvertTo-Json -Depth 2 -Compress
+'@
+    }
+
+    It 'defines its user-facing strings in exactly one place' {
+        # en-US/Messages.psd1 is the only place a string may be written. A second copy inside the
+        # module file cannot be kept in step by hand, which is the whole of #185.
+        $moduleSource = Get-Content -Path $script:sourceModulePath -Raw
+        $moduleSource | Should -Not -Match 'ConvertFrom-StringData'
+    }
+
+    It 'ships every string the module asks for' {
+        # The guard above keeps a second copy from existing. This one keeps the single copy
+        # complete, which is the other half of the same promise and fails differently: a key
+        # the module reads but Messages.psd1 never defines resolves to $null, and $null -f
+        # $argument is an empty string rather than an error. The message simply comes out
+        # blank -- on every host and every culture, not just the ones the Context below
+        # covers.
+        #
+        # That is not hypothetical. Publish-PSBuildModule read PathDoesNotExist from a
+        # ValidateScript on a mandatory parameter while Messages.psd1 never defined it, so
+        # passing a path that did not exist failed validation with no text at all, while the
+        # very next check in the same script block reported itself properly.
+        $sourceFile = Get-ChildItem -Path (
+            [IO.Path]::Combine($script:repositoryRoot, 'PowerShellBuild')
+        ) -Include '*.ps1', '*.psm1' -File -Recurse
+
+        $referencedKey = @(
+            $sourceFile |
+                Select-String -Pattern '\$LocalizedData\.(\w+)' -AllMatches |
+                ForEach-Object { $_.Matches } |
+                ForEach-Object { $_.Groups[1].Value } |
+                Sort-Object -Unique
+        )
+        $referencedKey | Should -Not -BeNullOrEmpty -Because 'the pattern must still match something'
+
+        $undefined = @($referencedKey.Where({ $_ -notin $script:shippedString.Keys }))
+        $undefined -join ', ' | Should -BeNullOrEmpty
+    }
+
+    Context 'Imported by <_.HostName> under a non-English UI culture' -ForEach $script:localizationHost {
+
+        BeforeAll {
+            # fr-FR is chosen because the module ships no fr-FR directory and none of its parents
+            # is en-US, so nothing but a deliberate fallback can reach the shipped strings.
+            $probeArgument = @(
+                '-NoProfile'
+                '-NonInteractive'
+                '-File', $script:probeScriptPath
+                '-ModuleFilePath', $script:builtModuleFilePath
+                '-UICulture', 'fr-FR'
+            )
+            $probeOutput = & $_.HostPath @probeArgument 2>&1
+
+            # The probe writes one JSON document and nothing else. Selecting the line rather than
+            # taking all output keeps a stray host warning from breaking the parse.
+            $probeJson = $probeOutput |
+                Where-Object { $_ -is [string] -and $_.TrimStart().StartsWith('{') } |
+                Select-Object -Last 1
+
+            $script:probeOutput = $probeOutput
+            $script:resolvedString = if ($probeJson) { $probeJson | ConvertFrom-Json }
+        }
+
+        It 'reports a string table from the probe' {
+            # Guards the two assertions below: a probe that produced nothing would otherwise
+            # satisfy a foreach over an empty set and prove nothing.
+            $script:resolvedString |
+                Should -Not -BeNullOrEmpty -Because "the probe wrote: $($script:probeOutput -join '; ')"
+        }
+
+        It 'binds every string the module ships' {
+            $resolvedKey = $script:resolvedString.PSObject.Properties.Name | Sort-Object
+            $shippedKey = $script:shippedString.Keys | Sort-Object
+            $resolvedKey | Should -Be $shippedKey
+        }
+
+        It 'binds each string to its shipped text' {
+            # Key-for-key rather than a count, because the failure this pins was a copy that had
+            # the right shape and the wrong words -- it named a Pester floor of 5.0.0 long after
+            # #182 raised the real one to 6.0.0.
+            foreach ($key in $script:shippedString.Keys) {
+                $script:resolvedString.$key |
+                    Should -Be $script:shippedString[$key] -Because "[$key] must match en-US/Messages.psd1"
+            }
+        }
+    }
+}