node.js Test Container Discoverer is active in non JavaScript projects
naelterman opened this issue · 3 comments
The NodeJS Test Container Discoverer is active in non JavaScript projects. In particular, it is active in C++ projects (.vcxproj).
I am talking about this class: Microsoft.NodejsTools.TestAdapter.TestContainerDiscoverer in this file.
When this class is queried, it iterates over all files and gets their last modified timestamp.
On large projects, this results in a non-neglectable delay. On our C++ code base, it takes between 30 seconds and 4 minutes.
Expected Behavior
NodeJSTools is not active in C++ projects and thus does not search for tests.
Actual Behavior
NodeJSTools tries to discover tests, producing disk IO for each file in the solution.
This disk IO takes more time on larger projects.
Up to 300 files, it is neglectable (<0.1 seconds).
For 1500 files, it is about half a second.
For 4500 files, it is about 2 seconds.
For 7500 files (attached solution), it is 3-5 seconds.
- NTVS Version: 1.5.10610.1 (Installed as part of the Node.js development workload)
- Visual Studio Version: 2019 (16.2.0)
- Node.js Version: -not relevant-
Steps to Reproduce
- Install Visual Studio 2019 with the Node,js development workload
- Download and extract LotOfFiles.zip
- Open Visual Studio
- Set the logging level for tests to 'Diagnostic' in Tools > Options > Test > General
- Open the solution (LotOfFiles/LotOfFiles.sln)
- Open the Test Explorer if you haven't already (default shortcut: Ctrl+E, T)
- Examine the test output in the 'Output' pane with the dropdown set to 'Tests'.
- Notice the text for the nodejs TestContainerDiscoverer is printed after a 3-5 seconds delay. (You might want to copy paste the output to a text editor and use search functionality.)
Solution
I've taken a look at the code and I believe this issue is rather simple to solve.
The following code is from TestContainerDiscoverer.cs lines 257-292:
public IEnumerable<ITestContainer> GetTestContainers(IVsProject project)
{
if (!project.TryGetProjectPath(out var path))
{
yield break;
}
// --- begin changes ---
// No need to search for tests in not supported projects.
if (!TypeScriptHelpers.IsSupportedTestProjectFile(path))
{
yield break;
}
// --- end changes ---
if (this.detectingChanges)
{
this.SaveModifiedFiles(project);
}
if (!this.knownProjects.TryGetValue(path, out var projectInfo) || !TryGetProjectUnitTestProperties(projectInfo.Project, out _, out _))
{
// Don't return any containers for projects we don't know about or that we know that they are not configured for JavaScript unit tests.
yield break;
}
projectInfo.HasRequestedContainers = true;
var latestWrite = project.GetProjectItemPaths().Aggregate(
this.lastWrite,
(latest, filePath) =>
{
try
{
var ft = File.GetLastWriteTimeUtc(filePath);
return (ft > latest) ? ft : latest;
}
catch (Exception exc) when (exc is UnauthorizedAccessException || exc is ArgumentException || exc is IOException)
{
}
return latest;
});
yield return new TestContainer(this, path, latestWrite);
}
I have not been able to test this proposed change as I cannot get the test container discoverer working when I build NodeJsTools locally.
It just doesn't run when the experimental instance Visual Studio instance is launched.
Thanks for recording this issue. I'm experiencing the same "non-negligible" delay and I have had to uninstall the Node.js development workload as a temporary workaround. The delay in my project was upward of 15 minutes.
This bug has been driving me crazy for a long time, and I'm glad to finally have found the cause for it.
For me, compared to trying to run a single test directly in Test Explorer just once, it literally (and not figuratively) takes less time to:
- Exit Visual Studio and a 100+ project solution
- Open Visual Studio Installer
- Uninstall NodeJS Development Tools
- Reopen Visual Studio and the big solution
- Build solution
- Wait for test-discovery to re-finish
- Click to run test
- Exit Visual Studio
- Open Visual Studio Installer
- Reinstall NodeJS Development Tools
- Reopen Visual Studio and the big solution
The amount of delay and lost man-hours this bug caused me in sizable projects is just unimaginable.
Please, pretty please with cherry on top, fix this bug.