Skip to content

Commit 0c8bdf4

Browse files
committed
Fix broken profile loading test
This change fixes the LanguageServerTests.ServiceLoadsProfilesOnDemand test which was broken by recent changes to our setting loading code. It removes a potential condition where sending a null string for the Script Analyzer settings path causes the AnalysisService to be restarted. This restart resulted in a load failure which polluted the console output, causing the expectations for the profile loading test to fail.
1 parent b7cf594 commit 0c8bdf4

File tree

3 files changed

+40
-20
lines changed

3 files changed

+40
-20
lines changed

src/PowerShellEditorServices.Protocol/Server/LanguageServer.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ protected async Task HandleDidChangeConfigurationNotification(
360360
// If there is a new settings file path, restart the analyzer with the new settigs.
361361
bool settingsPathChanged = false;
362362
string newSettingsPath = this.currentSettings.ScriptAnalysis.SettingsPath;
363-
if (!(oldScriptAnalysisSettingsPath?.Equals(newSettingsPath, StringComparison.OrdinalIgnoreCase) ?? false))
363+
if (!string.Equals(oldScriptAnalysisSettingsPath, newSettingsPath, StringComparison.OrdinalIgnoreCase))
364364
{
365365
this.editorSession.RestartAnalysisService(newSettingsPath);
366366
settingsPathChanged = true;

src/PowerShellEditorServices.Protocol/Server/LanguageServerSettings.cs

+16-3
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ public void Update(ScriptAnalysisSettings settings, string workspaceRootPath)
4444
{
4545
if (settings != null)
4646
{
47-
Validate.IsNotNullOrEmptyString(nameof(workspaceRootPath), workspaceRootPath);
48-
4947
this.Enable = settings.Enable;
5048

5149
string settingsPath = settings.SettingsPath;
@@ -56,7 +54,22 @@ public void Update(ScriptAnalysisSettings settings, string workspaceRootPath)
5654
}
5755
else if (!Path.IsPathRooted(settingsPath))
5856
{
59-
settingsPath = Path.GetFullPath(Path.Combine(workspaceRootPath, settingsPath));
57+
if (string.IsNullOrEmpty(workspaceRootPath))
58+
{
59+
// The workspace root path could be an empty string
60+
// when the user has opened a PowerShell script file
61+
// without opening an entire folder (workspace) first.
62+
// In this case we should just log an error and let
63+
// the specified settings path go through even though
64+
// it will fail to load.
65+
Logger.Write(
66+
LogLevel.Error,
67+
"Could not resolve Script Analyzer settings path due to null or empty workspaceRootPath.");
68+
}
69+
else
70+
{
71+
settingsPath = Path.GetFullPath(Path.Combine(workspaceRootPath, settingsPath));
72+
}
6073
}
6174

6275
this.SettingsPath = settingsPath;

test/PowerShellEditorServices.Test.Host/LanguageServerTests.cs

+23-16
Original file line numberDiff line numberDiff line change
@@ -619,21 +619,6 @@ public async Task ServiceExecutesNativeCommandAndReceivesCommand()
619619
[Fact]
620620
public async Task ServiceLoadsProfilesOnDemand()
621621
{
622-
// Send the configuration change to cause profiles to be loaded
623-
await this.languageServiceClient.SendEvent(
624-
DidChangeConfigurationNotification<LanguageServerSettingsWrapper>.Type,
625-
new DidChangeConfigurationParams<LanguageServerSettingsWrapper>
626-
{
627-
Settings = new LanguageServerSettingsWrapper
628-
{
629-
Powershell = new LanguageServerSettings
630-
{
631-
EnableProfileLoading = true,
632-
ScriptAnalysis = null
633-
}
634-
}
635-
});
636-
637622
string testProfilePath =
638623
Path.GetFullPath(
639624
@"..\..\..\PowerShellEditorServices.Test.Shared\Profile\Profile.ps1");
@@ -654,6 +639,28 @@ await this.languageServiceClient.SendEvent(
654639
// Copy the test profile to the current user's host profile path
655640
File.Copy(testProfilePath, currentUserCurrentHostPath, true);
656641

642+
Assert.True(
643+
File.Exists(currentUserCurrentHostPath),
644+
"Copied profile path does not exist!");
645+
646+
// Send the configuration change to cause profiles to be loaded
647+
await this.languageServiceClient.SendEvent(
648+
DidChangeConfigurationNotification<LanguageServerSettingsWrapper>.Type,
649+
new DidChangeConfigurationParams<LanguageServerSettingsWrapper>
650+
{
651+
Settings = new LanguageServerSettingsWrapper
652+
{
653+
Powershell = new LanguageServerSettings
654+
{
655+
EnableProfileLoading = true,
656+
ScriptAnalysis = new ScriptAnalysisSettings
657+
{
658+
Enable = false
659+
}
660+
}
661+
}
662+
});
663+
657664
OutputReader outputReader = new OutputReader(this.protocolClient);
658665

659666
Task<EvaluateResponseBody> evaluateTask =
@@ -665,7 +672,7 @@ await this.languageServiceClient.SendEvent(
665672
Context = "repl"
666673
});
667674

668-
// Try reading up to 10 lines to find the
675+
// Try reading up to 10 lines to find the expected output line
669676
string outputString = null;
670677
for (int i = 0; i < 10; i++)
671678
{

0 commit comments

Comments
 (0)