Skip to content

Commit 803367e

Browse files
EvangelinkEvangelink
authored
Simplify handling traits/categories and include categories in test context properties (#7349)
2 parents 83c36df + 5c99167 commit 803367e

File tree

23 files changed

+147
-608
lines changed

23 files changed

+147
-608
lines changed

‎src/Adapter/MSTestAdapter.PlatformServices/Execution/TestExecutionManager.cs‎

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,7 @@ private async Task ExecuteTestsWithTestRunnerAsync(
486486

487487
// Run single test passing test context properties to it.
488488
IDictionary<TestProperty, object?>? tcmProperties = TcmTestPropertiesProvider.GetTcmProperties(currentTest);
489-
Dictionary<string, object?> testContextProperties = GetTestContextProperties(tcmProperties, sourceLevelParameters);
489+
Dictionary<string, object?> testContextProperties = GetTestContextProperties(tcmProperties, sourceLevelParameters, unitTestElement);
490490

491491
TestTools.UnitTesting.TestResult[] unitTestResult;
492492
if (usesAppDomains || Thread.CurrentThread.GetApartmentState() == ApartmentState.STA)
@@ -522,24 +522,31 @@ private async Task ExecuteTestsWithTestRunnerAsync(
522522
/// </summary>
523523
/// <param name="tcmProperties">Tcm properties.</param>
524524
/// <param name="sourceLevelParameters">Source level parameters.</param>
525+
/// <param name="unitTestElement">The unit test element to get properties from.</param>
525526
/// <returns>Test context properties.</returns>
526527
private static Dictionary<string, object?> GetTestContextProperties(
527528
IDictionary<TestProperty, object?>? tcmProperties,
528-
IDictionary<string, object> sourceLevelParameters)
529+
IDictionary<string, object> sourceLevelParameters,
530+
UnitTestElement unitTestElement)
529531
{
530-
if (tcmProperties is null)
532+
// If we only have sourceLevelParameters, we create a new dictionary with just those.
533+
if (tcmProperties is null &&
534+
unitTestElement.Traits is null or { Length: 0 } &&
535+
unitTestElement.TestCategory is null or { Length: 0 })
531536
{
532537
return [with(sourceLevelParameters!)];
533538
}
534539

535-
// This dictionary will have *at least* 8 entries. Those are the sourceLevelParameters
536-
// which were originally calculated from TestDeployment.GetDeploymentInformation.
537-
var testContextProperties = new Dictionary<string, object?>(capacity: 8);
540+
// To avoid any resizes and additional overhead, we calculate the capacity beforehand.
541+
var testContextProperties = new Dictionary<string, object?>(capacity: sourceLevelParameters.Count + (tcmProperties?.Count ?? 0) + (unitTestElement.Traits?.Length ?? 0) + (unitTestElement.TestCategory?.Length ?? 0));
538542

539543
// Add tcm properties.
540-
foreach ((TestProperty key, object? value) in tcmProperties)
544+
if (tcmProperties is not null)
541545
{
542-
testContextProperties[key.Id] = value;
546+
foreach ((TestProperty key, object? value) in tcmProperties)
547+
{
548+
testContextProperties[key.Id] = value;
549+
}
543550
}
544551

545552
// Add source level parameters.
@@ -548,9 +555,54 @@ private async Task ExecuteTestsWithTestRunnerAsync(
548555
testContextProperties[key] = value;
549556
}
550557

558+
if (unitTestElement.Traits is { Length: > 0 })
559+
{
560+
foreach (Trait trait in unitTestElement.Traits)
561+
{
562+
ValidateAndAssignTestProperty(testContextProperties, trait.Name, trait.Value);
563+
}
564+
}
565+
566+
if (unitTestElement.TestCategory is { Length: > 0 })
567+
{
568+
foreach (string category in unitTestElement.TestCategory)
569+
{
570+
ValidateAndAssignTestProperty(testContextProperties, category, string.Empty);
571+
}
572+
}
573+
551574
return testContextProperties;
552575
}
553576

577+
/// <summary>
578+
/// Validates If a Custom test property is valid and then adds it to the TestContext property list.
579+
/// </summary>
580+
/// <param name="testContextProperties"> The test context properties. </param>
581+
/// <param name="propertyName"> The property name. </param>
582+
/// <param name="propertyValue"> The property value. </param>
583+
private static void ValidateAndAssignTestProperty(
584+
Dictionary<string, object?> testContextProperties,
585+
string propertyName,
586+
string propertyValue)
587+
{
588+
if (StringEx.IsNullOrEmpty(propertyName))
589+
{
590+
return;
591+
}
592+
593+
if (testContextProperties.ContainsKey(propertyName))
594+
{
595+
// Do not add to the test context because it would conflict with an already existing value.
596+
// We were at one point reporting a warning here. However with extensibility centered around TestProperty where
597+
// users can have multiple WorkItemAttributes(say) we cannot throw a warning here. Users would have multiple of these attributes
598+
// so that it shows up in reporting rather than seeing them in TestContext properties.
599+
}
600+
else
601+
{
602+
testContextProperties.Add(propertyName, propertyValue);
603+
}
604+
}
605+
554606
[SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "Requirement is to handle errors in user specified run parameters")]
555607
private void CacheSessionParameters(IRunContext? runContext, ITestExecutionRecorder testExecutionRecorder)
556608
{

‎src/Adapter/MSTestAdapter.PlatformServices/Execution/TestMethodInfo.cs‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,6 @@ internal TestMethodInfo(
6767
/// </summary>
6868
public bool IsTimeoutSet => TimeoutInfo.Timeout != TimeoutWhenNotSet;
6969

70-
/// <summary>
71-
/// Gets or sets the reason why the test is not runnable.
72-
/// </summary>
73-
public string? NotRunnableReason { get; internal set; }
74-
75-
/// <summary>
76-
/// Gets a value indicating whether test is runnable.
77-
/// </summary>
78-
public bool IsRunnable => StringEx.IsNullOrEmpty(NotRunnableReason);
79-
8070
/// <summary>
8171
/// Gets the parameter types of the test method.
8272
/// </summary>

‎src/Adapter/MSTestAdapter.PlatformServices/Execution/TypeCache.cs‎

Lines changed: 1 addition & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,6 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
1818
/// </summary>
1919
internal sealed class TypeCache : MarshalByRefObject
2020
{
21-
/// <summary>
22-
/// Predefined test Attribute names.
23-
/// </summary>
24-
private static readonly string[] PredefinedNames = ["Priority", "TestCategory", "Owner"];
25-
2621
/// <summary>
2722
/// Helper for reflection API's.
2823
/// </summary>
@@ -644,11 +639,7 @@ private TestMethodInfo ResolveTestMethodInfo(TestMethod testMethod, TestClassInf
644639

645640
MethodInfo methodInfo = GetMethodInfoForTestMethod(testMethod, testClassInfo);
646641

647-
var testMethodInfo = new TestMethodInfo(methodInfo, testClassInfo, testContext);
648-
649-
SetCustomProperties(testMethodInfo, testContext);
650-
651-
return testMethodInfo;
642+
return new TestMethodInfo(methodInfo, testClassInfo, testContext);
652643
}
653644

654645
private DiscoveryTestMethodInfo ResolveTestMethodInfoForDiscovery(TestMethod testMethod, TestClassInfo testClassInfo)
@@ -707,91 +698,4 @@ private MethodInfo GetMethodInfoForTestMethod(TestMethod testMethod, TestClassIn
707698
? null
708699
: testMethodInfo;
709700
}
710-
711-
/// <summary>
712-
/// Set custom properties.
713-
/// </summary>
714-
/// <param name="testMethodInfo"> The test Method Info. </param>
715-
/// <param name="testContext"> The test Context. </param>
716-
private void SetCustomProperties(TestMethodInfo testMethodInfo, ITestContext testContext)
717-
{
718-
DebugEx.Assert(testMethodInfo != null, "testMethodInfo is Null");
719-
DebugEx.Assert(testMethodInfo.MethodInfo != null, "testMethodInfo.TestMethod is Null");
720-
721-
// Avoid calling GetAttributes<T> to prevent iterator state machine allocations.
722-
_ = ValidateAttributes(_reflectionHelper.GetCustomAttributesCached(testMethodInfo.MethodInfo), testMethodInfo, testContext) &&
723-
ValidateAttributes(_reflectionHelper.GetCustomAttributesCached(testMethodInfo.Parent.ClassType), testMethodInfo, testContext);
724-
725-
static bool ValidateAttributes(Attribute[] attributes, TestMethodInfo testMethodInfo, ITestContext testContext)
726-
{
727-
foreach (Attribute attribute in attributes)
728-
{
729-
if (attribute is not TestPropertyAttribute testPropertyAttribute)
730-
{
731-
continue;
732-
}
733-
734-
if (!ValidateAndAssignTestProperty(testMethodInfo, testContext, testPropertyAttribute.Name, testPropertyAttribute.Value, isPredefinedAttribute: attribute is OwnerAttribute or PriorityAttribute))
735-
{
736-
return false;
737-
}
738-
}
739-
740-
return true;
741-
}
742-
}
743-
744-
/// <summary>
745-
/// Validates If a Custom test property is valid and then adds it to the TestContext property list.
746-
/// </summary>
747-
/// <param name="testMethodInfo"> The test method info. </param>
748-
/// <param name="testContext"> The test context. </param>
749-
/// <param name="propertyName"> The property name. </param>
750-
/// <param name="propertyValue"> The property value. </param>
751-
/// <param name="isPredefinedAttribute"> If the property originates from a predefined attribute. </param>
752-
/// <returns> True if its a valid Test Property. </returns>
753-
private static bool ValidateAndAssignTestProperty(
754-
TestMethodInfo testMethodInfo,
755-
ITestContext testContext,
756-
string propertyName,
757-
string propertyValue,
758-
bool isPredefinedAttribute)
759-
{
760-
if (!isPredefinedAttribute && PredefinedNames.Any(predefinedProp => predefinedProp == propertyName))
761-
{
762-
testMethodInfo.NotRunnableReason = string.Format(
763-
CultureInfo.CurrentCulture,
764-
Resource.UTA_ErrorPredefinedTestProperty,
765-
testMethodInfo.MethodInfo.DeclaringType!.FullName,
766-
testMethodInfo.MethodInfo.Name,
767-
propertyName);
768-
769-
return false;
770-
}
771-
772-
if (StringEx.IsNullOrEmpty(propertyName))
773-
{
774-
testMethodInfo.NotRunnableReason = string.Format(
775-
CultureInfo.CurrentCulture,
776-
Resource.UTA_ErrorTestPropertyNullOrEmpty,
777-
testMethodInfo.MethodInfo.DeclaringType!.FullName,
778-
testMethodInfo.MethodInfo.Name);
779-
780-
return false;
781-
}
782-
783-
if (testContext.TryGetPropertyValue(propertyName, out object? existingValue))
784-
{
785-
// Do not add to the test context because it would conflict with an already existing value.
786-
// We were at one point reporting a warning here. However with extensibility centered around TestProperty where
787-
// users can have multiple WorkItemAttributes(say) we cannot throw a warning here. Users would have multiple of these attributes
788-
// so that it shows up in reporting rather than seeing them in TestContext properties.
789-
}
790-
else
791-
{
792-
testContext.AddProperty(propertyName, propertyValue);
793-
}
794-
795-
return true;
796-
}
797701
}

‎src/Adapter/MSTestAdapter.PlatformServices/Execution/UnitTestRunner.cs‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -299,22 +299,6 @@ private static bool IsTestMethodRunnable(
299299
}
300300
}
301301

302-
// If test cannot be executed, then bail out.
303-
if (!testMethodInfo.IsRunnable)
304-
{
305-
{
306-
notRunnableResult =
307-
[
308-
new TestResult
309-
{
310-
Outcome = UnitTestOutcome.NotRunnable,
311-
IgnoreReason = testMethodInfo.NotRunnableReason,
312-
},
313-
];
314-
return false;
315-
}
316-
}
317-
318302
bool shouldIgnoreClass = testMethodInfo.Parent.ClassType.IsIgnored(out string? ignoreMessageOnClass);
319303
bool shouldIgnoreMethod = testMethodInfo.MethodInfo.IsIgnored(out string? ignoreMessageOnMethod);
320304

‎src/Adapter/MSTestAdapter.PlatformServices/Resources/Resource.resx‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -380,15 +380,9 @@ but received {4} argument(s), with types '{5}'.</value>
380380
<data name="UTA_ErrorNonPublicTestClass" xml:space="preserve">
381381
<value>UTA001: TestClass attribute defined on non-public class {0}</value>
382382
</data>
383-
<data name="UTA_ErrorPredefinedTestProperty" xml:space="preserve">
384-
<value>UTA023: {0}: Cannot define predefined property {2} on method {1}.</value>
385-
</data>
386383
<data name="UTA_ErrorTestClassIsGenericNonAbstract" xml:space="preserve">
387384
<value>TestClass attribute defined on generic non-abstract class {0}</value>
388385
</data>
389-
<data name="UTA_ErrorTestPropertyNullOrEmpty" xml:space="preserve">
390-
<value>UTA021: {0}: Null or empty custom property defined on method {1}. The custom property must have a valid name.</value>
391-
</data>
392386
<data name="UTA_ExecuteThrewException" xml:space="preserve">
393387
<value>An unhandled exception was thrown by the 'Execute' method. Please report this error to the author of the attribute '{0}'.
394388
{1}</value>

‎src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.cs.xlf‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -435,21 +435,11 @@ byl však přijat tento počet argumentů: {4} s typy {5}.</target>
435435
<target state="translated">UTA001: Atribut TestClass se definoval v neveřejné třídě {0}.</target>
436436
<note />
437437
</trans-unit>
438-
<trans-unit id="UTA_ErrorPredefinedTestProperty">
439-
<source>UTA023: {0}: Cannot define predefined property {2} on method {1}.</source>
440-
<target state="translated">UTA023: {0}: V metodě {1} nejde definovat předdefinovanou vlastnost {2}.</target>
441-
<note />
442-
</trans-unit>
443438
<trans-unit id="UTA_ErrorTestClassIsGenericNonAbstract">
444439
<source>TestClass attribute defined on generic non-abstract class {0}</source>
445440
<target state="translated">Atribut TestClass definovaný u obecné neabstraktní třídy {0}</target>
446441
<note />
447442
</trans-unit>
448-
<trans-unit id="UTA_ErrorTestPropertyNullOrEmpty">
449-
<source>UTA021: {0}: Null or empty custom property defined on method {1}. The custom property must have a valid name.</source>
450-
<target state="translated">UTA021: {0}: V metodě {1} je definovaná vlastní vlastnost, která je null nebo je prázdná. Vlastní vlastnost musí mít platný název.</target>
451-
<note />
452-
</trans-unit>
453443
<trans-unit id="UTA_ExecuteThrewException">
454444
<source>An unhandled exception was thrown by the 'Execute' method. Please report this error to the author of the attribute '{0}'.
455445
{1}</source>

‎src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.de.xlf‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -435,21 +435,11 @@ aber empfing {4} Argument(e) mit den Typen „{5}“.</target>
435435
<target state="translated">UTA001: Für die nicht öffentliche Klasse '{0}' definiertes Attribut 'TestClass'.</target>
436436
<note />
437437
</trans-unit>
438-
<trans-unit id="UTA_ErrorPredefinedTestProperty">
439-
<source>UTA023: {0}: Cannot define predefined property {2} on method {1}.</source>
440-
<target state="translated">UTA023: {0}: Die vordefinierte Eigenschaft "{2}" kann nicht für die Methode "{1}" definiert werden.</target>
441-
<note />
442-
</trans-unit>
443438
<trans-unit id="UTA_ErrorTestClassIsGenericNonAbstract">
444439
<source>TestClass attribute defined on generic non-abstract class {0}</source>
445440
<target state="translated">TestClass-Attribut für generische nicht abstrakte Klasse {0} definiert</target>
446441
<note />
447442
</trans-unit>
448-
<trans-unit id="UTA_ErrorTestPropertyNullOrEmpty">
449-
<source>UTA021: {0}: Null or empty custom property defined on method {1}. The custom property must have a valid name.</source>
450-
<target state="translated">UTA021: {0}: Für die Methode "{1}" wurde eine benutzerdefinierte Eigenschaft mit dem Wert NULL oder eine benutzerdefinierte leere Eigenschaft definiert. Die benutzerdefinierte Eigenschaft muss einen gültigen Namen aufweisen.</target>
451-
<note />
452-
</trans-unit>
453443
<trans-unit id="UTA_ExecuteThrewException">
454444
<source>An unhandled exception was thrown by the 'Execute' method. Please report this error to the author of the attribute '{0}'.
455445
{1}</source>

‎src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.es.xlf‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -435,21 +435,11 @@ pero recibió {4} argumentos, con los tipos '{5}'.</target>
435435
<target state="translated">UTA001: se ha definido el atributo TestClass en la clase no pública {0}</target>
436436
<note />
437437
</trans-unit>
438-
<trans-unit id="UTA_ErrorPredefinedTestProperty">
439-
<source>UTA023: {0}: Cannot define predefined property {2} on method {1}.</source>
440-
<target state="translated">UTA023: {0}: no se puede definir la propiedad predefinida {2} en el método {1}.</target>
441-
<note />
442-
</trans-unit>
443438
<trans-unit id="UTA_ErrorTestClassIsGenericNonAbstract">
444439
<source>TestClass attribute defined on generic non-abstract class {0}</source>
445440
<target state="translated">Atributo TestClass definido en una clase genérica no abstracta {0}</target>
446441
<note />
447442
</trans-unit>
448-
<trans-unit id="UTA_ErrorTestPropertyNullOrEmpty">
449-
<source>UTA021: {0}: Null or empty custom property defined on method {1}. The custom property must have a valid name.</source>
450-
<target state="translated">UTA021: {0}: se ha definido una propiedad personalizada nula o vacía en el método {1}. La propiedad personalizada debe tener un nombre válido.</target>
451-
<note />
452-
</trans-unit>
453443
<trans-unit id="UTA_ExecuteThrewException">
454444
<source>An unhandled exception was thrown by the 'Execute' method. Please report this error to the author of the attribute '{0}'.
455445
{1}</source>

‎src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.fr.xlf‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -435,21 +435,11 @@ mais a reçu {4} argument(s), avec les types « {5} ».</target>
435435
<target state="translated">UTA001 : attribut TestClass défini sur la classe non publique {0}</target>
436436
<note />
437437
</trans-unit>
438-
<trans-unit id="UTA_ErrorPredefinedTestProperty">
439-
<source>UTA023: {0}: Cannot define predefined property {2} on method {1}.</source>
440-
<target state="translated">UTA023 : {0} : Impossible de définir la propriété prédéfinie {2} sur la méthode {1}.</target>
441-
<note />
442-
</trans-unit>
443438
<trans-unit id="UTA_ErrorTestClassIsGenericNonAbstract">
444439
<source>TestClass attribute defined on generic non-abstract class {0}</source>
445440
<target state="translated">Attribut TestClass défini sur une classe non abstraite générique {0}</target>
446441
<note />
447442
</trans-unit>
448-
<trans-unit id="UTA_ErrorTestPropertyNullOrEmpty">
449-
<source>UTA021: {0}: Null or empty custom property defined on method {1}. The custom property must have a valid name.</source>
450-
<target state="translated">UTA021 : {0} : Une propriété null ou vide personnalisée est définie sur la méthode {1}. La propriété personnalisée doit posséder un nom valide.</target>
451-
<note />
452-
</trans-unit>
453443
<trans-unit id="UTA_ExecuteThrewException">
454444
<source>An unhandled exception was thrown by the 'Execute' method. Please report this error to the author of the attribute '{0}'.
455445
{1}</source>

‎src/Adapter/MSTestAdapter.PlatformServices/Resources/xlf/Resource.it.xlf‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -435,21 +435,11 @@ ma ha ricevuto {4} argomenti, con tipi '{5}'.</target>
435435
<target state="translated">UTA001: è stato definito l'attributo TestClass per la classe non pubblica {0}</target>
436436
<note />
437437
</trans-unit>
438-
<trans-unit id="UTA_ErrorPredefinedTestProperty">
439-
<source>UTA023: {0}: Cannot define predefined property {2} on method {1}.</source>
440-
<target state="translated">UTA023: {0}: non è possibile definire la proprietà predefinita {2} per il metodo {1}.</target>
441-
<note />
442-
</trans-unit>
443438
<trans-unit id="UTA_ErrorTestClassIsGenericNonAbstract">
444439
<source>TestClass attribute defined on generic non-abstract class {0}</source>
445440
<target state="translated">Attributo TestClass definito nella classe generica non astratta {0}</target>
446441
<note />
447442
</trans-unit>
448-
<trans-unit id="UTA_ErrorTestPropertyNullOrEmpty">
449-
<source>UTA021: {0}: Null or empty custom property defined on method {1}. The custom property must have a valid name.</source>
450-
<target state="translated">UTA021: {0}: per il metodo {1} è stata definita una proprietà personalizzata Null o vuota. Specificare un nome valido per la proprietà personalizzata.</target>
451-
<note />
452-
</trans-unit>
453443
<trans-unit id="UTA_ExecuteThrewException">
454444
<source>An unhandled exception was thrown by the 'Execute' method. Please report this error to the author of the attribute '{0}'.
455445
{1}</source>

0 commit comments

Comments
 (0)