node.js Test Container Discoverer is active in non JavaScript projects

Aperta
#2,256 3 commenti 1 reazione 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
3/5
Tempo stimato
1-2 giorni
Idoneità per principianti
38/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Ferma
Stack tecnologico
csharp, node.js

Direzione di ricerca

Inizia in Nodejs/Product/TestAdapterImpl/TestContainerDiscoverer.cs, in GetTestContainers, e analizza il controllo esistente TypeScriptHelpers.IsSupportedTestProjectFile proposto nell’issue. Riproduci il problema con LotOfFiles/LotOfFiles.sln e l’output diagnostico di Test Explorer; il lavoro è completato quando i progetti C++ non attivano più la discovery né i controlli dei timestamp per file, mentre i progetti JavaScript supportati continuano a farlo.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

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
  1. Install Visual Studio 2019 with the Node,js development workload
  2. Download and extract LotOfFiles.zip
  3. Open Visual Studio
  4. Set the logging level for tests to 'Diagnostic' in Tools > Options > Test > General
  5. Open the solution (LotOfFiles/LotOfFiles.sln)
  6. Open the Test Explorer if you haven't already (default shortcut: Ctrl+E, T)
  7. Examine the test output in the 'Output' pane with the dropdown set to 'Tests'.
  8. 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.

Lingua principale
C#
Stelle
1.8k
Fork
355
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Guida per i contributori

Apri la guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di microsoft/nodejstools

Tutte le issue di microsoft/nodejstools

Issue simili

Altre issue su C#

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.