Run NetFramework tests inside vstest.console.exe process #1009

Faizan2304 · 2017-08-18T17:29:36Z

Run test within vstest.console.exe process for project targeting NetFramework if running from commandline.
This will be the default behavior. To opt out default behavior user has to explicitly pass '/InIsolation' argument.

2) Wrote unit tests.

Faizan2304 · 2017-08-18T18:21:45Z

Please see the matrix below to see the benefit of using InProcess argument.

Performance matrix

Number of tests	Normal Flow (in seconds)	InProcess (in seconds)	Percentage benefit
10 tests in one project	1.20 seconds	0.42 seconds	65%
100 tests in one project	1.23 seconds	0.45 seconds	63.4 %
1000 tests in one project	1.73 seconds	0.76 seconds	56%
10000 tests in one project	9.15 seconds	4.76 seconds	47.9%
10000 tests in 10 projects	9.91 seconds	6.42 seconds	35.2%
1 lakh test in 10 projects	75.60 seconds	46.60 seconds	38.3%

@pvlakshm @sbaid @codito

mayankbansal018 · 2017-08-21T05:29:40Z

src/Microsoft.TestPlatform.CrossPlatEngine/Client/InProcessProxyexecutionManager.cs

+            {
+                EqtTrace.Error("InProcessProxyexecutionManager.StartTestRun: Failed to start test run: {0}", exception);
+
+                // Send a discovery complete to caller.


nit: change comment to test execution complete

Fixed in next commit.

mayankbansal018 · 2017-08-21T05:31:58Z

src/Microsoft.TestPlatform.CrossPlatEngine/TestEngine.cs

@@ -59,14 +63,19 @@ public IProxyDiscoveryManager GetDiscoveryManager(ITestRuntimeProvider testHostM
        {
            var parallelLevel = this.VerifyParallelSettingAndCalculateParallelLevel(discoveryCriteria.Sources.Count(), discoveryCriteria.RunSettings);


this can be moved below

Fixed in next commit.

mayankbansal018 · 2017-08-21T05:35:49Z

src/Microsoft.TestPlatform.CrossPlatEngine/TestEngine.cs

+            }
+
+            var runConfiguration = XmlRunSettingsUtilities.GetRunConfigurationNode(runsettings);
+


Add a comment specifying when will we enable InProc run

smadala · 2017-08-22T18:02:28Z

src/Microsoft.TestPlatform.CrossPlatEngine/Client/InProcessProxyDiscoveryManager.cs

+    using Microsoft.VisualStudio.TestPlatform.ObjectModel.Engine.TesthostProtocol;
+    using Microsoft.VisualStudio.TestPlatform.ObjectModel.Logging;
+
+    class InProcessProxyDiscoveryManager : IProxyDiscoveryManager


public or internal class?

smadala · 2017-08-22T18:07:10Z

src/Microsoft.TestPlatform.CrossPlatEngine/Client/InProcessProxyDiscoveryManager.cs

+                    EqtTrace.Error("InProcessProxyDiscoveryManager.DiscoverTests: Failed to discover tests: {0}", exception);
+
+                    // Send a discovery complete to caller.
+                    eventHandler.HandleLogMessage(TestMessageLevel.Error, exception.Message);


Log exception stacktrace also on failure scenario.

smadala · 2017-08-22T18:08:51Z

src/Microsoft.TestPlatform.CrossPlatEngine/Client/InProcessProxyexecutionManager.cs

+            }
+        }
+
+        public int StartTestRun(TestRunCriteria testRunCriteria, ITestRunEventsHandler eventHandler)


smadala · 2017-08-22T18:32:14Z

src/Microsoft.TestPlatform.CrossPlatEngine/TestEngine.cs

+            if (runConfiguration.InProcess &&
+                !runConfiguration.DisableAppDomain &&
+                !runConfiguration.DesignMode &&
+                !(runConfiguration.TargetFrameworkVersion.Name.IndexOf("netstandard", StringComparison.OrdinalIgnoreCase) >= 0


Use Name.StartsWith()?
What scenario the netstandard is being used as TargetFramework?
If we target netstandard2.0, can enable this scenarion for .net core?

smadala · 2017-08-22T18:42:54Z

src/Microsoft.TestPlatform.CrossPlatEngine/TestEngine.cs

+                !(runConfiguration.TargetFrameworkVersion.Name.IndexOf("netstandard", StringComparison.OrdinalIgnoreCase) >= 0
+                || runConfiguration.TargetFrameworkVersion.Name.IndexOf("netcoreapp", StringComparison.OrdinalIgnoreCase) >= 0))
+            {
+                return true ;


Log eqttrace message on successful/unsuccessful using of inproc. Give warning on using inproc option for not supported target frameworks.

smadala · 2017-08-22T18:44:04Z

src/Microsoft.TestPlatform.ObjectModel/RunSettings/RunConfiguration.cs

@@ -376,6 +399,15 @@ public bool DisableAppDomainSet
        }

        /// <summary>
+        /// Gets a value indicating whether InProcess is set.
+        /// </summary>
+        public bool InProcessSet


Where is this used?

smadala · 2017-08-22T19:08:10Z

src/Microsoft.TestPlatform.CrossPlatEngine/Client/InProcessProxyDiscoveryManager.cs

+        /// <param name="eventHandler">EventHandler for handling discovery events from Engine</param>
+        public void DiscoverTests(DiscoveryCriteria discoveryCriteria, ITestDiscoveryEventsHandler eventHandler)
+        {
+            var discoveryManager = this.testHostManagerFactory.GetDiscoveryManager();


Why calling GetDiscoveryManager() in every method, Can be a member variable?

and we can call Initilize() instead of doing check below.

smadala · 2017-08-22T19:14:47Z

...crosoft.TestPlatform.CrossPlatEngine.UnitTests/Client/InProcessProxyDiscoveryManagerTests.cs

+        public void InitializeShouldCallDiscoveryManagerInitializeWithEmptyIEnumerable()
+        {
+            this.inProcessProxyDiscoveryManager.Initialize();
+            this.mockDiscoveryManager.Verify(o => o.Initialize(Enumerable.Empty<string>()), Times.Once);


Add assert failure message for all asserts.

Fix in next commit.

chakradhar2 · 2017-08-24T13:04:51Z

src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscoveryManager.cs

-            // Start using these additional extensions
-            TestPluginCache.Instance.DefaultExtensionPaths = pathToAdditionalExtensions;
+            if (pathToAdditionalExtensions != null && pathToAdditionalExtensions.Count() > 0)
+            {


Can we use pathToAdditionalExtensions.Any() instead of pathToAdditionalExtensions.Count() > 0?

Yes we can use

chakradhar2 · 2017-08-24T13:21:56Z

src/Microsoft.TestPlatform.CrossPlatEngine/Client/InProcessProxyDiscoveryManager.cs

+        /// </summary>
+        public void Abort()
+        {
+            Task.Run(() => this.testHostManagerFactory.GetDiscoveryManager().Abort());


Do we require any null check for this.testHostManagerFactory?

No, we don't require. If its null, process should catch exception

2) Enable InIsolation argument to disable default in process. 3) Address PR comment

codito · 2017-08-28T11:58:19Z

src/Microsoft.TestPlatform.CrossPlatEngine/Client/InProcessProxyDiscoveryManager.cs

+    internal class InProcessProxyDiscoveryManager : IProxyDiscoveryManager
+    {
+        private ITestHostManagerFactory testHostManagerFactory;
+        IDiscoveryManager discoveryManager;


Explicit annotation for types.

Fixed in next commit.

codito · 2017-08-28T12:12:57Z

src/Microsoft.TestPlatform.CrossPlatEngine/Client/InProcessProxyDiscoveryManager.cs

+
+                    // Send a discovery complete to caller.
+                    eventHandler.HandleLogMessage(TestMessageLevel.Error, exception.ToString());
+                    eventHandler.HandleDiscoveryComplete(-1, new List<TestCase>(), true);


Enumerable.Empty<TestCase>

codito · 2017-08-28T12:16:55Z

src/Microsoft.TestPlatform.CrossPlatEngine/Client/InProcessProxyexecutionManager.cs

+                var executionContext = new TestExecutionContext(
+                            testRunCriteria.FrequencyOfRunStatsChangeEvent,
+                            testRunCriteria.RunStatsChangeEventTimeout,
+                            inIsolation: false,


How is this used in TPv1? Are we missing scenario?

This info get passed to adapter through RunContext

codito · 2017-08-28T12:30:37Z

src/Microsoft.TestPlatform.CrossPlatEngine/Client/InProcessProxyexecutionManager.cs

+
+                if (testRunCriteria.HasSpecificSources)
+                {
+                    // [TODO]: we need to revisit to second-last argument if we will enable datacollector. 


nit: remove todo

codito · 2017-08-28T12:49:11Z

src/Microsoft.TestPlatform.CrossPlatEngine/Client/InProcessProxyexecutionManager.cs

+                            hasTestRun: true,
+                            isDebug: (testRunCriteria.TestHostLauncher != null && testRunCriteria.TestHostLauncher.IsDebug),
+                            testCaseFilter: testRunCriteria.TestCaseFilter);
+


We should ask TestRuntimeProvider for extensions.

Yes, we should.

faahmad and others added 6 commits July 27, 2017 14:56

No isolation discovery for net46 with design mode off

e98195a

NoIsolation test run for net46

ddb91be

Merge branch 'master' into noIsolation

bfb18d6

Make api asynchronous

a881a1a

Merge branch 'master' into noIsolation

a2a391d

1) Renamed argument processor to InProcess

ede0e04

2) Wrote unit tests.

Faizan2304 requested review from codito, smadala, harshjain2 and mayankbansal018 August 18, 2017 17:29

msftclas added the cla-not-required label Aug 18, 2017

mayankbansal018 approved these changes Aug 21, 2017

View reviewed changes

smadala suggested changes Aug 22, 2017

View reviewed changes

chakradhar2 reviewed Aug 24, 2017

View reviewed changes

faahmad added 3 commits August 25, 2017 20:43

1) Making in process default for net46

2f76f3c

2) Enable InIsolation argument to disable default in process. 3) Address PR comment

Merge branch 'master' into noIsolation

d1237cd

Disable inprocess for inProcDataCollector.

172f9d0

codito approved these changes Aug 28, 2017

View reviewed changes

faahmad added 3 commits August 29, 2017 00:04

Addressed Arun comment

39c40c2

Append comment

ed5a9ca

Merge branch 'master' into noIsolation

4124fde

Faizan2304 merged commit f205078 into microsoft:master Aug 28, 2017

Faizan2304 deleted the noIsolation branch August 28, 2017 20:43

		@@ -59,14 +63,19 @@ public IProxyDiscoveryManager GetDiscoveryManager(ITestRuntimeProvider testHostM
		{
		var parallelLevel = this.VerifyParallelSettingAndCalculateParallelLevel(discoveryCriteria.Sources.Count(), discoveryCriteria.RunSettings);

		}

		var runConfiguration = XmlRunSettingsUtilities.GetRunConfigurationNode(runsettings);

Run NetFramework tests inside vstest.console.exe process #1009

Run NetFramework tests inside vstest.console.exe process #1009

Conversation

Faizan2304 commented Aug 18, 2017 • edited Loading

Faizan2304 commented Aug 18, 2017 • edited Loading

Performance matrix

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Faizan2304 commented Aug 18, 2017 •

edited

Loading

Faizan2304 commented Aug 18, 2017 •

edited

Loading