Skip to content

Refactor tests #7702

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.

Sign up for GitHub

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

Open
hassan20990 wants to merge 2 commits into
base: main
Choose a base branch
from
+166 −71
Open

Refactor tests #7702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3c34c2
Fix the BuildSolution integration test
AdamYoblick Jun 16, 2022
f8cf469
Create settings.json
hassan20990 Aug 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"marquee.widgets.npm-stats.packageNames": [
"ptvs"
]
}
Comment on lines +1 to +5

@rchiodo rchiodo Jul 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This file appears to be unrelated scope creep in a "Refactor tests" PR — it only sets a personal Marquee npm-stats widget config and is unrelated to test reliability. Consider dropping it from the change. (It is also missing a trailing newline.)

4 changes: 4 additions & 0 deletions Common/Tests/Utilities.UI/TestUtilities.UI.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,10 @@
<Project>{D092D54E-FF29-4D32-9AEE-4EF704C92F67}</Project>
<Name>TestUtilities</Name>
</ProjectReference>
<ProjectReference Include="..\..\..\Python\Product\VSCommon\VSCommon.csproj">
<Project>{a52ac77f-6df9-4387-bb08-8ded3fd95a0f}</Project>
<Name>VSCommon</Name>
</ProjectReference>
<ProjectReference Include="..\..\..\Python\Tests\TestRunnerInterop\TestRunnerInterop.csproj">
<Project>{3e8c485b-905a-4dbc-b059-161ccb1ad591}</Project>
<Name>TestRunnerInterop</Name>
Expand Down
8 changes: 7 additions & 1 deletion Common/Tests/Utilities.UI/UI/TreeView.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Text.RegularExpressions;
using System.Windows.Automation;

namespace TestUtilities.UI {
Expand Down Expand Up @@ -92,7 +93,12 @@ public AutomationElement FindItem(params string[] path) {
protected static AutomationElement FindNode(AutomationElementCollection nodes, string[] splitPath, int depth) {
for (int i = 0; i < nodes.Count; i++) {
var node = nodes[i];
var name = node.GetCurrentPropertyValue(AutomationElement.NameProperty) as string;
var name = (node.GetCurrentPropertyValue(AutomationElement.NameProperty) as string);

// Sometimes AutomationElement.NameProperty contains non-printable characters that mess up the
// string compare, so get rid of those.
// See https://stackoverflow.com/questions/40564692/c-sharp-regex-to-remove-non-printable-characters-and-control-characters-in-a
name = Regex.Replace(name, @"\p{C}+", string.Empty).Trim();
Comment on lines +96 to +101

if (name.Equals(splitPath[depth], StringComparison.CurrentCulture)) {
if (depth == splitPath.Length - 1) {
Expand Down
206 changes: 148 additions & 58 deletions Common/Tests/Utilities.UI/UI/VisualStudioApp.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
using Microsoft.VisualStudio.Shell;
using Microsoft.VisualStudio.Shell.Interop;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Microsoft.VisualStudio.Threading;
using Microsoft.VisualStudioTools;
using IOleServiceProvider = Microsoft.VisualStudio.OLE.Interop.IServiceProvider;
Comment on lines 35 to 38
using Task = System.Threading.Tasks.Task;

Expand Down Expand Up @@ -834,6 +836,7 @@ public virtual Project CreateProject(
private static IEnumerable<IVsProject> EnumerateLoadedProjects(IVsSolution solution) {
IEnumHierarchies hierarchies;
Guid guid = Guid.Empty;

ErrorHandler.ThrowOnFailure((solution.GetProjectEnum(
(uint)(__VSENUMPROJFLAGS.EPF_ALLINSOLUTION),
ref guid,
Expand Down Expand Up @@ -888,45 +891,63 @@ public Project OpenProject(
bool setStartupItem = true,
Func<AutomationDialog, bool> onDialog = null
) {
// NOTE: In general, everything from the IVs* APIs need to run on the UI thread
// See the RunOnUIThread() and CreateTaskOnUIThread() helper methods
var solution = GetService<IVsSolution>(typeof(SVsSolution));
var solution4 = solution as IVsSolution4;
Assert.IsNotNull(solution, "Failed to obtain SVsSolution service");
Assert.IsNotNull(solution4, "Failed to obtain IVsSolution4 interface");

// Close any open solution
if (ErrorHandler.Succeeded(solution.GetSolutionInfo(out _, out string slnFile, out _))) {
Console.WriteLine("Closing {0}", slnFile);
solution.CloseSolutionElement(0, null, 0);
}
RunOnUIThread(() =>
{
// Close any open solution
if (ErrorHandler.Succeeded(solution.GetSolutionInfo(out _, out string slnFile, out _)))
{
if (!string.IsNullOrEmpty(slnFile)) {
Console.WriteLine("Closing {0}", slnFile);
solution.CloseSolutionElement(0, null, 0);
}
}
});

string fullPath = TestData.GetPath(projName);
if (!File.Exists(fullPath)) {
if (!File.Exists(fullPath))
{
Assert.Fail("Cannot find " + fullPath);
}

Console.WriteLine("Opening {0}", fullPath);

// If there is a .suo file, delete that so that there is no state carried over from another test.
for (int i = 10; i <= 15; ++i) {
for (int i = 10; i <= 15; ++i)
{
string suoPath = Path.ChangeExtension(fullPath, ".v" + i + ".suo");
if (File.Exists(suoPath)) {
if (File.Exists(suoPath))
{
File.Delete(suoPath);
}
}
var dotVsPath = Path.Combine(Path.GetDirectoryName(fullPath), ".vs");
if (Directory.Exists(dotVsPath)) {
foreach (var suoPath in FileUtils.EnumerateFiles(dotVsPath, ".vso")) {
if (Directory.Exists(dotVsPath))
{
foreach (var suoPath in FileUtils.EnumerateFiles(dotVsPath, ".vso"))
{
File.Delete(suoPath);
}
}

Task t;
if (fullPath.EndsWith(".sln", StringComparison.OrdinalIgnoreCase)) {
t = Task.Run(() => {
if (fullPath.EndsWith(".sln", StringComparison.OrdinalIgnoreCase))
{
t = CreateTaskOnUIThread(async () =>
{
ErrorHandler.ThrowOnFailure(solution.OpenSolutionFile((uint)0, fullPath));
});
Comment on lines +942 to 945
} else {
t = Task.Run(() => {
}
else
{
t = CreateTaskOnUIThread(async () =>
{

@rchiodo rchiodo Jul 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These async () => { ... } lambdas contain no await, producing CS1998 ("runs synchronously"). Harmless at runtime since the body executes on the UI thread after SwitchToMainThreadAsync() inside the helper, but if the harness project sets TreatWarningsAsErrors the build breaks. Drop the async keyword (return Task.CompletedTask) to silence the warning.

Guid guidNull = Guid.Empty;
Guid iidUnknown = Guid.Empty;
IntPtr projPtr;
Comment on lines +949 to 953
Expand All @@ -943,18 +964,26 @@ out projPtr
));
});
}
using (var cts = System.Diagnostics.Debugger.IsAttached ? new CancellationTokenSource() : new CancellationTokenSource(30000)) {
try {
if (!t.Wait(1000, cts.Token)) {

using (var cts = System.Diagnostics.Debugger.IsAttached ? new CancellationTokenSource() : new CancellationTokenSource(30000))
{
try
{
if (!t.Wait(1000, cts.Token))

@rchiodo rchiodo Jul 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CreateTaskOnUIThread returns the raw .Task from a JoinableTask, and the caller blocks with t.Wait(1000, cts.Token). Extracting and blocking on JoinableTask.Task bypasses JTF's joinable-collection deadlock mitigation. It's safe today only because OpenProject runs off the UI thread and the lambda bodies are synchronous; if OpenProject is ever invoked on the UI thread, the scheduled main-thread continuation can't run while Wait() blocks that thread, deadlocking until the 30s CTS fires. Prefer returning the JoinableTask and driving the dialog-poll loop via Join()/JoinAsync.

{
// Load has taken a while, start checking whether a dialog has
// appeared
IVsUIShell uiShell = GetService<IVsUIShell>(typeof(IVsUIShell));
IntPtr hwnd;
while (!t.Wait(1000, cts.Token)) {
while (!t.Wait(1000, cts.Token))
{
ErrorHandler.ThrowOnFailure(uiShell.GetDialogOwnerHwnd(out hwnd));

@rchiodo rchiodo Jul 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The threading fix is incomplete: the new comment states "everything from the IVs* APIs need to run on the UI thread," but IVsUIShell.GetDialogOwnerHwnd here — and vsProject.GetProperty at ~1057 — still run on the calling background thread while their neighbors (GetSolutionInfo, EnumerateLoadedProjects, GetGuidOfProject, ReloadProject) were wrapped in RunOnUIThread. Off-thread IVs* calls can throw RPC_E_WRONG_THREAD or silently COM-marshal. Either wrap these in RunOnUIThread too, or narrow the "everything" wording to match reality.

if (hwnd != _mainWindowHandle) {
using (var dlg = new AutomationDialog(this, AutomationElement.FromHandle(hwnd))) {
if (onDialog == null || onDialog(dlg) == false) {
if (hwnd != _mainWindowHandle)
{
using (var dlg = new AutomationDialog(this, AutomationElement.FromHandle(hwnd)))
{
if (onDialog == null || onDialog(dlg) == false)
{
Console.WriteLine("Unexpected dialog");
DumpElement(dlg.Element);
Assert.Fail("Unexpected dialog while loading project");
Expand All @@ -963,73 +992,102 @@ out projPtr
}
}
}
} catch (OperationCanceledException) {
}
catch (OperationCanceledException)
{
Assert.Fail("Failed to open project quickly enough");
}
}

var projects = EnumerateLoadedProjects(solution).ToList();
IEnumerable<IVsProject> projects = new List<IVsProject>();
RunOnUIThread(() =>
{
projects = EnumerateLoadedProjects(solution).ToList();

if (expectedProjects != null && expectedProjects.Value != projects.Count())
{
// if we have other files open we can end up with a bonus project...
Assert.AreEqual(
expectedProjects,
projects.Count(p =>
{
string mk;
return ErrorHandler.Succeeded(solution.GetUniqueNameOfProject((IVsHierarchy)p, out mk)) &&
mk != "Miscellaneous Files";
}),
"Wrong number of loaded projects"
);
}
});

if (expectedProjects != null && expectedProjects.Value != projects.Count) {
// if we have other files open we can end up with a bonus project...
Assert.AreEqual(
expectedProjects,
projects.Count(p => {
IVsHierarchy vsProject = null;
RunOnUIThread(() =>
{
vsProject = string.IsNullOrEmpty(projectName) ?
projects.OfType<IVsHierarchy>().FirstOrDefault() :
projects.OfType<IVsHierarchy>().FirstOrDefault(p =>
{
string mk;
return ErrorHandler.Succeeded(solution.GetUniqueNameOfProject((IVsHierarchy)p, out mk)) &&
mk != "Miscellaneous Files";
}),
"Wrong number of loaded projects"
);
}


var vsProject = string.IsNullOrEmpty(projectName) ?
projects.OfType<IVsHierarchy>().FirstOrDefault() :
projects.OfType<IVsHierarchy>().FirstOrDefault(p => {
string mk;
if (ErrorHandler.Failed(solution.GetUniqueNameOfProject(p, out mk))) {
return false;
}
Console.WriteLine(mk);
return Path.GetFileNameWithoutExtension(mk) == projectName;
});
if (ErrorHandler.Failed(solution.GetUniqueNameOfProject(p, out mk)))
{
return false;
}
Console.WriteLine(mk);
return Path.GetFileNameWithoutExtension(mk) == projectName;
});
});

string outputText = "(unable to get Solution output)";
try {
try
{
outputText = GetOutputWindowText("Solution");
} catch (Exception) {
}
catch (Exception)
{
}
Assert.IsNotNull(vsProject, "No project loaded: " + outputText);

Guid projGuid;
ErrorHandler.ThrowOnFailure(solution.GetGuidOfProject(vsProject, out projGuid));
Guid projGuid = Guid.Empty;
RunOnUIThread(() =>
{
ErrorHandler.ThrowOnFailure(solution.GetGuidOfProject(vsProject, out projGuid));
});

object o;
ErrorHandler.ThrowOnFailure(vsProject.GetProperty(VSConstants.VSITEMID_ROOT, (int)__VSHPROPID.VSHPROPID_ExtObject, out o));
var project = (Project)o;
Comment on lines 1056 to 1058

if (project.Properties == null) {
ErrorHandler.ThrowOnFailure(solution4.ReloadProject(ref projGuid));
if (project.Properties == null)
{
RunOnUIThread(() =>
{
ErrorHandler.ThrowOnFailure(solution4.ReloadProject(ref projGuid));
});
}

// HACK: Testing whether Properties is just slow to initialize
for (int retries = 10; retries > 0 && project.Properties == null; --retries) {
for (int retries = 10; retries > 0 && project.Properties == null; --retries)
{
Trace.TraceWarning("Waiting for project.Properties to become non-null");
System.Threading.Thread.Sleep(250);
}
Assert.IsNotNull(project.Properties, "No project properties: " + outputText);
Assert.IsTrue(project.Properties.GetEnumerator().MoveNext(), "No items in project properties: " + outputText);

if (startItem != null && setStartupItem) {
if (startItem != null && setStartupItem)
{
project.SetStartupFile(startItem);
for (var i = 0; i < 20; i++) {
for (var i = 0; i < 20; i++)
{
//Wait for the startupItem to be set before returning from the project creation
try {
if (((string)project.Properties.Item("StartupFile").Value) == startItem) {
try
{
if (((string)project.Properties.Item("StartupFile").Value) == startItem)
{
break;
}
} catch { }
}
catch { }
System.Threading.Thread.Sleep(250);
}
}
Expand Down Expand Up @@ -1120,5 +1178,37 @@ internal ProjectItem AddItem(Project project, string language, string template,
var fullTemplate = ((Solution2)project.DTE.Solution).GetProjectItemTemplate(template, language);
return project.ProjectItems.AddFromTemplate(fullTemplate, filename);
}

private static void RunOnUIThread(Action action) {

ThreadHelper.JoinableTaskFactory.Run(async () =>
{
// switch to the UI thread if we need to
if (!ThreadHelper.CheckAccess()) {
await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();
}

// now that we're on the UI thread, run the specified code
action();
});
}

private static Task CreateTaskOnUIThread(Func<Task> task)
{
var t = ThreadHelper.JoinableTaskFactory.RunAsync(async () =>
{
// switch to the UI thread if we need to
if (!ThreadHelper.CheckAccess())
{
await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync();
}

// now that we're on the UI thread, run the specified code
await task.Invoke();

}).Task;

return t;
}
}
}
14 changes: 2 additions & 12 deletions Common/Tests/Utilities.UI/UI/VisualStudioInstance.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,18 +157,8 @@ public string SolutionDirectory {

private string SolutionNodeText {
get {
if (_solution.Projects.Count(sln => !sln.Flags.HasFlag(SolutionElementFlags.ExcludeFromConfiguration) && !sln.Flags.HasFlag(SolutionElementFlags.ExcludeFromSolution)) > 1) {
return String.Format(
"Solution '{0}' ({1} of {1} projects)",
Path.GetFileNameWithoutExtension(_solution.Filename),
_solution.Projects.Length
);
}
return String.Format(
"Solution '{0}' (1 of 1 project)",
Path.GetFileNameWithoutExtension(_solution.Filename)
);

var solutionName = Path.GetFileNameWithoutExtension(_solution.Filename);
return $"Solution '{solutionName}'";
Comment on lines +160 to +161
}
}

Expand Down