-
Notifications
You must be signed in to change notification settings - Fork 269
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.
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
Adding support for DiaNavigation in UWP test adapter #258
Adding support for DiaNavigation in UWP test adapter #258
Conversation
Testing Verifed that for UWP test running via Tpv1, n exception is thrown Once we take dependency on Tpv2 Obj Model, we can enable commented testcases
@mayankbansal018, |
try | ||
{ | ||
Assembly objectModelAssembly = Assembly.Load(new AssemblyName("Microsoft.VisualStudio.TestPlatform.ObjectModel")); | ||
Type diaType = objectModelAssembly.GetType("Microsoft.VisualStudio.TestPlatform.ObjectModel.DiaSession"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we cache?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed, since this should be invoked only once when someone is trying to create DiaSesion, & then hold the session object.
Further the session is dependent based on for what exe/dll it is opened for, so caching would be wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It gets called for each assembly. I meant the GetType reflection call. You can still go ahead and create a new object each time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes the type object I'll cache
@@ -91,6 +104,21 @@ public void GetNavigationData(object navigationSession, string className, string | |||
// Initiate values and bail out. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: remove.
@@ -91,6 +104,21 @@ public void GetNavigationData(object navigationSession, string className, string | |||
// Initiate values and bail out. | |||
fileName = null; | |||
minLineNumber = -1; | |||
|
|||
MethodInfo methodInfo = navigationSession.GetType().GetMethod("GetNavigationData"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
caching here too maybe?
@@ -117,6 +117,57 @@ public void GetNavigationDataShouldReturnNullFileName() | |||
Assert.AreEqual(-1, minLineNumber); | |||
} | |||
|
|||
// Enable these tests when we take dependency on TpV2 object model | |||
// In Tpv1 UWP Object model these below methods are not defined. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would we ever do this given that we want to support UWP in 14.0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If not, how do we run these tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we would move to Tpv2 object model, once we have made it default. Right now I do not want to go into problem of acquiring it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm.. This is a debt we are carrying forward. Please file a bug to track this.
private static PropertyInfo propertyMinLineNumber; | ||
private static Type typeDiaSession; | ||
private static Type typeDiaNavigationData; | ||
|
||
/// <summary> | ||
/// Initializes static members of the <see cref="FileOperations"/> class. | ||
/// </summary> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the initialization now move into the new operations class? Doesn't make much sense for this to be aware of diasession type anymore.
#pragma warning disable SA1649 // SA1649FileNameMustMatchTypeName | ||
|
||
[TestClass] | ||
public class DiaSessionOperationsTests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for getting these enabled. Can we close this issue now: #261
Testing:
Verifed that for UWP test running via Tpv1, no exception is thrown(Since Dia Session is not available there)
Once we take dependency on Tpv2 Obj Model, we can enable commented testcases