Adapter Failed to Load #958 #2156
Conversation
|
Pushed fixes for failing tests, kindly verify and share your feedback |
| var pathToAdditionalExtensions = this.dataSerializer.DeserializePayload<IEnumerable<string>>(message); | ||
| jobQueue.QueueJob( | ||
| () => | ||
| testHostManagerFactory.GetDiscoveryManager().Initialize(pathToAdditionalExtensions), 0); | ||
| testHostManagerFactory.GetDiscoveryManager().Initialize(pathToAdditionalExtensions, discoveryEventsHandler), 0); |
There was a problem hiding this comment.
discoveryEventsHandler [](start = 116, length = 22)
should be testExtensionInitializeEventsHandler
There was a problem hiding this comment.
I will go ahead with this once we finalize the permanent fix for it
|
@singhsarab , @mayankbansal018 Overriding the TestMessageLevel From Error to Warning in ExecutionManager.cs and DiscoveryManager.cs files to get through the build checks. |
|
Pushed changes @singhsarab @mayankbansal018 |
| namespace Microsoft.VisualStudio.TestPlatform.Common | ||
| { | ||
| using System.Collections.Generic; |
There was a problem hiding this comment.
this seems unnecessary.
There was a problem hiding this comment.
has been removed
| @@ -316,12 +316,12 @@ private static void LogWarningOnNoTestsDiscovered(IEnumerable<string> sources, s | |||
| } | |||
| else | |||
| { | |||
| //Should be logged only when tests found - if adapter failed to load then there would be no tests to process | |||
| this.sessionMessageLogger.TestRunMessage += this.TestSessionMessageHandler; | ||
| } | ||
|
|
||
| private void TestSessionMessageHandler(object sender, TestRunMessageEventArgs e) |
There was a problem hiding this comment.
move private to bottom
There was a problem hiding this comment.
moved under private methods region
| @@ -1,6 +1,8 @@ | |||
| // Copyright (c) Microsoft Corporation. All rights reserved. | |||
| // Licensed under the MIT license. See LICENSE file in the project root for full license information. | |||
|
|
|||
|
|
|||
|
|
|||
| namespace Microsoft.VisualStudio.TestPlatform.Common | |||
There was a problem hiding this comment.
why is this file changed
| @@ -247,6 +249,11 @@ private void TestSessionMessageHandler(object sender, TestRunMessageEventArgs e) | |||
|
|
|||
| if (this.testDiscoveryEventsHandler != null) | |||
| { | |||
| if (e.Level == TestMessageLevel.Error) | |||
There was a problem hiding this comment.
Error [](start = 48, length = 5)
Do not downgrade error to warnings, this handler is also invoked when Adapters(Extensions) send something back as error, we cannot reduce their messages as warning.
There was a problem hiding this comment.
Removed Level downgrade lines, and now it follows whatever the Level it gets.
| @@ -88,6 +93,8 @@ public void Initialize(IEnumerable<string> pathToAdditionalExtensions) | |||
| ITestCaseEventsHandler testCaseEventsHandler, | |||
| ITestRunEventsHandler runEventsHandler) | |||
| { | |||
| //unsubscrive session logger | |||
| this.sessionMessageLogger.TestRunMessage -= this.TestSessionMessageHandler; | |||
There was a problem hiding this comment.
this should be done in Initialize itself
There was a problem hiding this comment.
Moved in to the Initialize()
| { | ||
| if (this.testMessageEventsHandler != null) | ||
| { | ||
| if (e.Level == TestMessageLevel.Error) |
There was a problem hiding this comment.
same as discovery handler
There was a problem hiding this comment.
Removed Level downgrade lines, and now it follows whatever the Level it gets.
| () => { }); | ||
|
|
||
| //Act | ||
| this.discoveryManager.Initialize(new List<string> { assemblyLocation }, mockLogger.Object); |
There was a problem hiding this comment.
assemblyLocation [](start = 64, length = 16)
make the message as appropriate
There was a problem hiding this comment.
Removed unnecessary test and updated message.
|
@mayankbansal018 , suggested changes pushed, kindly have a look |
|
Reverted unnecessary changes. |
No errors reported for adapters that are not loaded because of 'copied from internet' bit on adapter binary
#958