Skip to content
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.

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

Fix handful of unit test issues and the intermittent IndexOutOfRangeException failures #10635

Merged
merged 7 commits into from
Jul 17, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ internal static class FilePathNormalizer

public static string NormalizeDirectory(string? directoryFilePath)
{
if (directoryFilePath.IsNullOrEmpty())
if (directoryFilePath.IsNullOrEmpty() || directoryFilePath == "/")
{
return "/";
}
Expand Down Expand Up @@ -76,7 +76,7 @@ public static string Normalize(string? filePath)
/// </summary>
public static string GetNormalizedDirectoryName(string? filePath)
{
if (filePath.IsNullOrEmpty())
if (filePath.IsNullOrEmpty() || filePath == "/")
{
return "/";
}
Expand Down Expand Up @@ -253,7 +253,8 @@ private static void NormalizeAndDedupeSlashes(Span<char> span, ref int charsWrit
{
writeSlot = '/';

if (Unsafe.Add(ref readSlot, 1) is '/' or '\\')
// Be careful not to read past the end of the span.
if (read < charsWritten - 1 && Unsafe.Add(ref readSlot, 1) is '/' or '\\')
{
// We found two slashes in a row. If we are at the start of the path,
// we we are at '\\network' paths, so want to leave them alone. Otherwise
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Microsoft.AspNetCore.Razor.Language.Components;
using Microsoft.AspNetCore.Razor.LanguageServer.Tooltip;
using Microsoft.AspNetCore.Razor.ProjectSystem;
using Microsoft.AspNetCore.Razor.Test.Common;
using Microsoft.AspNetCore.Razor.Test.Common.LanguageServer;
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
using Roslyn.Test.Utilities;
Expand Down Expand Up @@ -374,7 +375,7 @@ await projectManager.UpdateAsync(updater =>
{
updater.ProjectAdded(hostProject);
updater.ProjectWorkspaceStateChanged(hostProject.Key, projectWorkspaceState);
updater.DocumentAdded(hostProject.Key, hostDocument, CreateTextLoader(hostDocument.FilePath, text: ""));
updater.DocumentAdded(hostProject.Key, hostDocument, TestMocks.CreateTextLoader(hostDocument.FilePath, text: ""));
});

var service = new TestTagHelperToolTipFactory(projectManager);
Expand Down Expand Up @@ -419,11 +420,11 @@ await projectManager.UpdateAsync(updater =>
{
updater.ProjectAdded(hostProject1);
updater.ProjectWorkspaceStateChanged(hostProject1.Key, projectWorkspaceState);
updater.DocumentAdded(hostProject1.Key, hostDocument, CreateTextLoader(hostDocument.FilePath, text: ""));
updater.DocumentAdded(hostProject1.Key, hostDocument, TestMocks.CreateTextLoader(hostDocument.FilePath, text: ""));

updater.ProjectAdded(hostProject2);
updater.ProjectWorkspaceStateChanged(hostProject2.Key, projectWorkspaceState);
updater.DocumentAdded(hostProject2.Key, hostDocument, CreateTextLoader(hostDocument.FilePath, text: ""));
updater.DocumentAdded(hostProject2.Key, hostDocument, TestMocks.CreateTextLoader(hostDocument.FilePath, text: ""));
});

var service = new TestTagHelperToolTipFactory(projectManager);
Expand Down Expand Up @@ -468,10 +469,10 @@ await projectManager.UpdateAsync(updater =>
{
updater.ProjectAdded(hostProject1);
updater.ProjectWorkspaceStateChanged(hostProject1.Key, projectWorkspaceState);
updater.DocumentAdded(hostProject1.Key, hostDocument, CreateTextLoader(hostDocument.FilePath, text: ""));
updater.DocumentAdded(hostProject1.Key, hostDocument, TestMocks.CreateTextLoader(hostDocument.FilePath, text: ""));

updater.ProjectAdded(hostProject2);
updater.DocumentAdded(hostProject2.Key, hostDocument, CreateTextLoader(hostDocument.FilePath, text: ""));
updater.DocumentAdded(hostProject2.Key, hostDocument, TestMocks.CreateTextLoader(hostDocument.FilePath, text: ""));
});

var service = new TestTagHelperToolTipFactory(projectManager);
Expand Down Expand Up @@ -512,10 +513,10 @@ public async Task GetAvailableProjects_NotAvailableInAnyProject_ReturnsText()
await projectManager.UpdateAsync(updater =>
{
updater.ProjectAdded(hostProject1);
updater.DocumentAdded(hostProject1.Key, hostDocument, CreateTextLoader(hostDocument.FilePath, text: ""));
updater.DocumentAdded(hostProject1.Key, hostDocument, TestMocks.CreateTextLoader(hostDocument.FilePath, text: ""));

updater.ProjectAdded(hostProject2);
updater.DocumentAdded(hostProject2.Key, hostDocument, CreateTextLoader(hostDocument.FilePath, text: ""));
updater.DocumentAdded(hostProject2.Key, hostDocument, TestMocks.CreateTextLoader(hostDocument.FilePath, text: ""));
});

var service = new TestTagHelperToolTipFactory(projectManager);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public async Task TryCreateForOpenDocumentAsync_CanNotResolveVersion_ReturnsNull

await _projectManager.UpdateAsync(updater =>
{
updater.DocumentAdded(MiscFilesHostProject.Instance.Key, hostDocument, CreateTextLoader(filePath, ""));
updater.DocumentAdded(MiscFilesHostProject.Instance.Key, hostDocument, TestMocks.CreateTextLoader(filePath, ""));
});

var factory = new DocumentContextFactory(_projectManager, _documentVersionCache, LoggerFactory);
Expand All @@ -87,7 +87,7 @@ public async Task TryCreateAsync_ResolvesContent()

await _projectManager.UpdateAsync(updater =>
{
updater.DocumentAdded(MiscFilesHostProject.Instance.Key, hostDocument, CreateTextLoader(filePath, ""));
updater.DocumentAdded(MiscFilesHostProject.Instance.Key, hostDocument, TestMocks.CreateTextLoader(filePath, ""));
});

var miscFilesProject = _projectManager.GetMiscellaneousProject();
Expand Down Expand Up @@ -142,7 +142,7 @@ public async Task TryCreateForOpenDocumentAsync_ResolvesContent()

await _projectManager.UpdateAsync(updater =>
{
updater.DocumentAdded(MiscFilesHostProject.Instance.Key, hostDocument, CreateTextLoader(filePath, ""));
updater.DocumentAdded(MiscFilesHostProject.Instance.Key, hostDocument, TestMocks.CreateTextLoader(filePath, ""));
});

var miscFilesProject = _projectManager.GetMiscellaneousProject();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
using Microsoft.CodeAnalysis.Text;
using Microsoft.CommonLanguageServerProtocol.Framework;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using Moq;
using Xunit.Abstractions;

namespace Microsoft.AspNetCore.Razor.Test.Common.LanguageServer;
Expand Down Expand Up @@ -131,23 +130,6 @@ private protected static VersionedDocumentContext CreateDocumentContext(Uri uri,
return new VersionedDocumentContext(uri, snapshot, projectContext: null, version: 0);
}

protected static TextLoader CreateTextLoader(string filePath, string text)
{
return CreateTextLoader(filePath, SourceText.From(text));
}

protected static TextLoader CreateTextLoader(string filePath, SourceText text)
{
var mock = new StrictMock<TextLoader>();

var textAndVersion = TextAndVersion.Create(text, VersionStamp.Create(), filePath);

mock.Setup(x => x.LoadTextAndVersionAsync(It.IsAny<LoadTextOptions>(), It.IsAny<CancellationToken>()))
.ReturnsAsync(textAndVersion);

return mock.Object;
}

private protected static RazorLSPOptionsMonitor GetOptionsMonitor(bool enableFormatting = true, bool autoShowCompletion = true, bool autoListParams = true, bool formatOnType = true, bool autoInsertAttributeQuotes = true, bool colorBackground = false, bool codeBlockBraceOnNextLine = false, bool commitElementsWithSpace = true)
{
var configService = StrictMock.Of<IConfigurationSyncService>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,11 @@ public void Log(LogLevel logLevel, string message, Exception? exception)
try
{
_provider.TestOutputHelper.WriteLine(finalMessage);
}
catch (InvalidOperationException iex) when (iex.Message == "There is no currently active test.")
{
// Ignore, something is logging a message outside of a test. Other loggers will capture it.

if (exception is not null)
{
_provider.TestOutputHelper.WriteLine(exception.ToString());
}
}
catch (Exception ex)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT license. See License.txt in the project root for license information.

using System.Threading;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Text;
using Moq;

namespace Microsoft.AspNetCore.Razor.Test.Common;

internal static class TestMocks
{
public static TextLoader CreateTextLoader(string filePath, string text)
{
return CreateTextLoader(filePath, SourceText.From(text));
}

public static TextLoader CreateTextLoader(string filePath, SourceText text)
{
var mock = new StrictMock<TextLoader>();

var textAndVersion = TextAndVersion.Create(text, VersionStamp.Create(), filePath);

mock.Setup(x => x.LoadTextAndVersionAsync(It.IsAny<LoadTextOptions>(), It.IsAny<CancellationToken>()))
.ReturnsAsync(textAndVersion);

return mock.Object;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,17 @@
using Microsoft.AspNetCore.Razor.ProjectSystem;
using Microsoft.AspNetCore.Razor.Test.Common;
using Microsoft.AspNetCore.Razor.Test.Common.VisualStudio;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Razor;
using Microsoft.CodeAnalysis.Razor.Logging;
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.Razor.DynamicFiles;
using Moq;
using Xunit;
using Xunit.Abstractions;

namespace Microsoft.CodeAnalysis.Razor.ProjectSystem;
namespace Microsoft.VisualStudio.Razor.ProjectSystem;

// These tests are really integration tests. There isn't a good way to unit test this functionality since
// the only thing in here is threading.
Expand Down Expand Up @@ -219,27 +222,32 @@ await projectManager.UpdateAsync(updater =>
[UIFact]
public async Task ProcessWorkAndRestart()
{
var hostProject1 = TestProjectData.SomeProject;
var hostProject2 = TestProjectData.AnotherProject;
var hostDocument1 = TestProjectData.SomeProjectFile1;
var hostDocument2 = TestProjectData.SomeProjectFile2;

// Arrange
var projectManager = CreateProjectSnapshotManager();

await projectManager.UpdateAsync(updater =>
{
updater.ProjectAdded(s_hostProject1);
updater.ProjectAdded(s_hostProject2);
updater.DocumentAdded(s_hostProject1.Key, s_documents[0], null!);
updater.DocumentAdded(s_hostProject1.Key, s_documents[1], null!);
updater.ProjectAdded(hostProject1);
updater.ProjectAdded(hostProject2);
updater.DocumentAdded(hostProject1.Key, hostDocument1, null!);
updater.DocumentAdded(hostProject1.Key, hostDocument2, null!);
});

var project = projectManager.GetLoadedProject(s_hostProject1.Key);
var documentKey1 = new DocumentKey(project.Key, s_documents[0].FilePath);
var documentKey2 = new DocumentKey(project.Key, s_documents[1].FilePath);
var project = projectManager.GetLoadedProject(hostProject1.Key);
var documentKey1 = new DocumentKey(project.Key, hostDocument1.FilePath);
var documentKey2 = new DocumentKey(project.Key, hostDocument2.FilePath);

using var generator = new TestBackgroundDocumentGenerator(projectManager, _dynamicFileInfoProvider, LoggerFactory);

// Act & Assert

// First, enqueue some work.
generator.Enqueue(project, project.GetDocument(s_documents[0].FilePath).AssumeNotNull());
generator.Enqueue(project, project.GetDocument(hostDocument1.FilePath).AssumeNotNull());

// Wait for the work to complete.
await generator.WaitUntilCurrentBatchCompletesAsync();
Expand All @@ -248,14 +256,14 @@ await projectManager.UpdateAsync(updater =>
Assert.Single(generator.CompletedWork, documentKey1);

// Enqueue more work.
generator.Enqueue(project, project.GetDocument(s_documents[1].FilePath).AssumeNotNull());
generator.Enqueue(project, project.GetDocument(hostDocument2.FilePath).AssumeNotNull());

// Wait for the work to complete.
await generator.WaitUntilCurrentBatchCompletesAsync();

Assert.Collection(generator.CompletedWork.OrderBy(key => key.DocumentFilePath),
key => Assert.Equal(documentKey2, key),
key => Assert.Equal(documentKey1, key));
key => Assert.Equal(documentKey1, key),
key => Assert.Equal(documentKey2, key));
}

[UIFact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ public async Task StopListening_UnadvisesForFileChange()
.Verifiable();
fileChangeService
.Setup(f => f.UnadviseFileChangeAsync(123, It.IsAny<CancellationToken>()))
.ReturnsAsync(TestProjectData.SomeProjectImportFile.FilePath)
.Verifiable();
var tracker = new VisualStudioFileChangeTracker(
TestProjectData.SomeProjectImportFile.FilePath,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.LanguageServer.ProjectSystem;
using Microsoft.AspNetCore.Razor.ProjectSystem;
using Microsoft.AspNetCore.Razor.Test.Common;
using Microsoft.AspNetCore.Razor.Test.Common.LanguageServer;
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
using Xunit;
Expand Down Expand Up @@ -41,10 +42,10 @@ public async Task ProcessesExistingProjectsDuringInitialization()
await projectManager.UpdateAsync(static updater =>
{
updater.ProjectAdded(s_hostProject1);
updater.DocumentAdded(s_hostProject1.Key, s_hostDocument1, CreateTextLoader(s_hostDocument1.FilePath, "<p>Hello World</p>"));
updater.DocumentAdded(s_hostProject1.Key, s_hostDocument1, TestMocks.CreateTextLoader(s_hostDocument1.FilePath, "<p>Hello World</p>"));

updater.ProjectAdded(s_hostProject2);
updater.DocumentAdded(s_hostProject2.Key, s_hostDocument2, CreateTextLoader(s_hostDocument2.FilePath, "<p>Hello World</p>"));
updater.DocumentAdded(s_hostProject2.Key, s_hostDocument2, TestMocks.CreateTextLoader(s_hostDocument2.FilePath, "<p>Hello World</p>"));
});

var (driver, testAccessor) = await CreateDriverAndInitializeAsync(projectManager);
Expand Down Expand Up @@ -93,10 +94,10 @@ public async Task ProcessesProjectsAddedAfterInitialization()
await projectManager.UpdateAsync(static updater =>
{
updater.ProjectAdded(s_hostProject1);
updater.DocumentAdded(s_hostProject1.Key, s_hostDocument1, CreateTextLoader(s_hostDocument1.FilePath, "<p>Hello World</p>"));
updater.DocumentAdded(s_hostProject1.Key, s_hostDocument1, TestMocks.CreateTextLoader(s_hostDocument1.FilePath, "<p>Hello World</p>"));

updater.ProjectAdded(s_hostProject2);
updater.DocumentAdded(s_hostProject2.Key, s_hostDocument2, CreateTextLoader(s_hostDocument2.FilePath, "<p>Hello World</p>"));
updater.DocumentAdded(s_hostProject2.Key, s_hostDocument2, TestMocks.CreateTextLoader(s_hostDocument2.FilePath, "<p>Hello World</p>"));
});

await testAccessor.WaitUntilCurrentBatchCompletesAsync();
Expand Down Expand Up @@ -134,7 +135,7 @@ await projectManager.UpdateAsync(static updater =>

await projectManager.UpdateAsync(static updater =>
{
updater.DocumentAdded(s_hostProject1.Key, s_hostDocument1, CreateTextLoader(s_hostDocument1.FilePath, "<p>Hello World</p>"));
updater.DocumentAdded(s_hostProject1.Key, s_hostDocument1, TestMocks.CreateTextLoader(s_hostDocument1.FilePath, "<p>Hello World</p>"));
});

await testAccessor.WaitUntilCurrentBatchCompletesAsync();
Expand Down Expand Up @@ -204,7 +205,7 @@ await projectManager.UpdateAsync(static updater =>

await projectManager.UpdateAsync(static updater =>
{
updater.DocumentAdded(s_hostProject1.Key, s_hostDocument1, CreateTextLoader(s_hostDocument1.FilePath, "<p>Hello World</p>"));
updater.DocumentAdded(s_hostProject1.Key, s_hostDocument1, TestMocks.CreateTextLoader(s_hostDocument1.FilePath, "<p>Hello World</p>"));
});

await testAccessor.WaitUntilCurrentBatchCompletesAsync();
Expand Down