Address review comment

Youssef1313 · Youssef1313 · commit 88d27b7ffe76 · 2024-11-19T10:05:54.000+01:00
diff --git a/src/TestFramework/TestFramework/Assertions/CollectionAssert.cs b/src/TestFramework/TestFramework/Assertions/CollectionAssert.cs
@@ -1158,8 +1158,7 @@ public static void AreEqual(ICollection? expected, ICollection? actual, [StringS
         string reason = string.Empty;
         if (!AreCollectionsEqual(expected, actual, new ObjectComparer(), ref reason))
         {
-            string userMessage = Assert.BuildUserMessage(message, parameters);
-            string finalMessage = string.Format(CultureInfo.CurrentCulture, FrameworkMessages.CollectionEqualReason, userMessage, reason);
+            string finalMessage = ConstructFinalMessage(reason, message, parameters);
             Assert.ThrowAssertFailed("CollectionAssert.AreEqual", finalMessage);
         }
     }
@@ -1243,8 +1242,7 @@ public static void AreNotEqual(ICollection? notExpected, ICollection? actual, [S
         string reason = string.Empty;
         if (AreCollectionsEqual(notExpected, actual, new ObjectComparer(), ref reason))
         {
-            string userMessage = Assert.BuildUserMessage(message, parameters);
-            string finalMessage = string.Format(CultureInfo.CurrentCulture, FrameworkMessages.CollectionEqualReason, userMessage, reason);
+            string finalMessage = ConstructFinalMessage(reason, message, parameters);
             Assert.ThrowAssertFailed("CollectionAssert.AreNotEqual", finalMessage);
         }
     }
@@ -1335,8 +1333,7 @@ public static void AreEqual(ICollection? expected, ICollection? actual, [NotNull
         string reason = string.Empty;
         if (!AreCollectionsEqual(expected, actual, comparer, ref reason))
         {
-            string userMessage = Assert.BuildUserMessage(message, parameters);
-            string finalMessage = string.Format(CultureInfo.CurrentCulture, FrameworkMessages.CollectionEqualReason, userMessage, reason);
+            string finalMessage = ConstructFinalMessage(reason, message, parameters);
             Assert.ThrowAssertFailed("CollectionAssert.AreEqual", finalMessage);
         }
     }
@@ -1427,8 +1424,7 @@ public static void AreNotEqual(ICollection? notExpected, ICollection? actual, [N
         string reason = string.Empty;
         if (AreCollectionsEqual(notExpected, actual, comparer, ref reason))
         {
-            string userMessage = Assert.BuildUserMessage(message, parameters);
-            string finalMessage = string.Format(CultureInfo.CurrentCulture, FrameworkMessages.CollectionEqualReason, userMessage, reason);
+            string finalMessage = ConstructFinalMessage(reason, message, parameters);
             Assert.ThrowAssertFailed("CollectionAssert.AreNotEqual", finalMessage);
         }
     }
@@ -1674,6 +1670,17 @@ private static bool CompareIEnumerable(IEnumerable? expected, IEnumerable? actua
         return true;
     }
 
+    private static string ConstructFinalMessage(
+        string reason,
+        [StringSyntax(StringSyntaxAttribute.CompositeFormat)] string? message,
+        params object?[]? parameters)
+    {
+        string userMessage = Assert.BuildUserMessage(message, parameters);
+        return userMessage.Length == 0
+            ? reason
+            : string.Format(CultureInfo.CurrentCulture, FrameworkMessages.CollectionEqualReason, userMessage, reason);
+    }
+
     /// <summary>
     /// compares the objects using object.Equals.
     /// </summary>
diff --git a/src/TestFramework/TestFramework/Resources/FrameworkMessages.Designer.cs b/src/TestFramework/TestFramework/Resources/FrameworkMessages.Designer.cs
diff --git a/src/TestFramework/TestFramework/Resources/FrameworkMessages.resx b/src/TestFramework/TestFramework/Resources/FrameworkMessages.resx
@@ -160,7 +160,7 @@
     <value>Both collections contain the same elements. {0}</value>
   </data>
   <data name="CollectionEqualReason" xml:space="preserve">
-    <value>{0}({1})</value>
+    <value>{0}. {1}</value>
   </data>
   <data name="ContainsFail" xml:space="preserve">
     <value>String '{0}' does not contain string '{1}'. {2}.</value>
diff --git a/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.cs.xlf b/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.cs.xlf
@@ -73,9 +73,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="CollectionEqualReason">
-        <source>{0}({1})</source>
-        <target state="translated">{0}({1})</target>
-        <note></note>
+        <source>{0}. {1}</source>
+        <target state="needs-review-translation">{0}({1})</target>
+        <note />
       </trans-unit>
       <trans-unit id="ContainsFail">
         <source>String '{0}' does not contain string '{1}'. {2}.</source>
diff --git a/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.de.xlf b/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.de.xlf
@@ -73,9 +73,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="CollectionEqualReason">
-        <source>{0}({1})</source>
-        <target state="translated">{0}({1})</target>
-        <note></note>
+        <source>{0}. {1}</source>
+        <target state="needs-review-translation">{0}({1})</target>
+        <note />
       </trans-unit>
       <trans-unit id="ContainsFail">
         <source>String '{0}' does not contain string '{1}'. {2}.</source>
diff --git a/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.es.xlf b/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.es.xlf
@@ -73,9 +73,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="CollectionEqualReason">
-        <source>{0}({1})</source>
-        <target state="translated">{0}({1})</target>
-        <note></note>
+        <source>{0}. {1}</source>
+        <target state="needs-review-translation">{0}({1})</target>
+        <note />
       </trans-unit>
       <trans-unit id="ContainsFail">
         <source>String '{0}' does not contain string '{1}'. {2}.</source>
diff --git a/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.fr.xlf b/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.fr.xlf
@@ -73,9 +73,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="CollectionEqualReason">
-        <source>{0}({1})</source>
-        <target state="translated">{0}({1})</target>
-        <note></note>
+        <source>{0}. {1}</source>
+        <target state="needs-review-translation">{0}({1})</target>
+        <note />
       </trans-unit>
       <trans-unit id="ContainsFail">
         <source>String '{0}' does not contain string '{1}'. {2}.</source>
diff --git a/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.it.xlf b/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.it.xlf
@@ -73,9 +73,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="CollectionEqualReason">
-        <source>{0}({1})</source>
-        <target state="translated">{0}({1})</target>
-        <note></note>
+        <source>{0}. {1}</source>
+        <target state="needs-review-translation">{0}({1})</target>
+        <note />
       </trans-unit>
       <trans-unit id="ContainsFail">
         <source>String '{0}' does not contain string '{1}'. {2}.</source>
diff --git a/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.ja.xlf b/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.ja.xlf
@@ -73,9 +73,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="CollectionEqualReason">
-        <source>{0}({1})</source>
-        <target state="translated">{0}({1})</target>
-        <note></note>
+        <source>{0}. {1}</source>
+        <target state="needs-review-translation">{0}({1})</target>
+        <note />
       </trans-unit>
       <trans-unit id="ContainsFail">
         <source>String '{0}' does not contain string '{1}'. {2}.</source>
diff --git a/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.ko.xlf b/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.ko.xlf
@@ -73,9 +73,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="CollectionEqualReason">
-        <source>{0}({1})</source>
-        <target state="translated">{0}({1})</target>
-        <note></note>
+        <source>{0}. {1}</source>
+        <target state="needs-review-translation">{0}({1})</target>
+        <note />
       </trans-unit>
       <trans-unit id="ContainsFail">
         <source>String '{0}' does not contain string '{1}'. {2}.</source>
diff --git a/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.pl.xlf b/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.pl.xlf
@@ -73,9 +73,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="CollectionEqualReason">
-        <source>{0}({1})</source>
-        <target state="translated">{0}({1})</target>
-        <note></note>
+        <source>{0}. {1}</source>
+        <target state="needs-review-translation">{0}({1})</target>
+        <note />
       </trans-unit>
       <trans-unit id="ContainsFail">
         <source>String '{0}' does not contain string '{1}'. {2}.</source>
diff --git a/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.pt-BR.xlf b/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.pt-BR.xlf
@@ -73,9 +73,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="CollectionEqualReason">
-        <source>{0}({1})</source>
-        <target state="translated">{0}({1})</target>
-        <note></note>
+        <source>{0}. {1}</source>
+        <target state="needs-review-translation">{0}({1})</target>
+        <note />
       </trans-unit>
       <trans-unit id="ContainsFail">
         <source>String '{0}' does not contain string '{1}'. {2}.</source>
diff --git a/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.ru.xlf b/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.ru.xlf
@@ -73,9 +73,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="CollectionEqualReason">
-        <source>{0}({1})</source>
-        <target state="translated">{0}({1})</target>
-        <note></note>
+        <source>{0}. {1}</source>
+        <target state="needs-review-translation">{0}({1})</target>
+        <note />
       </trans-unit>
       <trans-unit id="ContainsFail">
         <source>String '{0}' does not contain string '{1}'. {2}.</source>
diff --git a/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.tr.xlf b/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.tr.xlf
@@ -73,9 +73,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="CollectionEqualReason">
-        <source>{0}({1})</source>
-        <target state="translated">{0}({1})</target>
-        <note></note>
+        <source>{0}. {1}</source>
+        <target state="needs-review-translation">{0}({1})</target>
+        <note />
       </trans-unit>
       <trans-unit id="ContainsFail">
         <source>String '{0}' does not contain string '{1}'. {2}.</source>
diff --git a/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.zh-Hans.xlf b/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.zh-Hans.xlf
@@ -73,9 +73,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="CollectionEqualReason">
-        <source>{0}({1})</source>
-        <target state="translated">{0}({1})</target>
-        <note></note>
+        <source>{0}. {1}</source>
+        <target state="needs-review-translation">{0}({1})</target>
+        <note />
       </trans-unit>
       <trans-unit id="ContainsFail">
         <source>String '{0}' does not contain string '{1}'. {2}.</source>
diff --git a/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.zh-Hant.xlf b/src/TestFramework/TestFramework/Resources/xlf/FrameworkMessages.zh-Hant.xlf
@@ -73,9 +73,9 @@
         <note></note>
       </trans-unit>
       <trans-unit id="CollectionEqualReason">
-        <source>{0}({1})</source>
-        <target state="translated">{0}({1})</target>
-        <note></note>
+        <source>{0}. {1}</source>
+        <target state="needs-review-translation">{0}({1})</target>
+        <note />
       </trans-unit>
       <trans-unit id="ContainsFail">
         <source>String '{0}' does not contain string '{1}'. {2}.</source>
diff --git a/test/UnitTests/TestFramework.UnitTests/Assertions/CollectionAssertTests.cs b/test/UnitTests/TestFramework.UnitTests/Assertions/CollectionAssertTests.cs
@@ -465,14 +465,25 @@ public void CollectionAssertAreNotEquivalent_FailsWithTwoNullsAndComparer_WithMe
         Verify(ex.Message.Contains("message"));
     }
 
-    public void CollectionAssertAreEqual_FailsWithGoodMessage()
+    public void CollectionAssertAreEqualWithoutUserMessage_FailsWithGoodMessage()
     {
         Exception ex = VerifyThrows(() => CollectionAssert.AreEqual(new[] { 1, 2, 3 }, new[] { 1, 5, 3 }));
         Assert.AreEqual(
             """
-            CollectionAssert.AreEqual failed. (Element at index 1 do not match.
+            CollectionAssert.AreEqual failed. Element at index 1 do not match.
             Expected: 2
-            Actual: 5)
+            Actual: 5
+            """, ex.Message);
+    }
+
+    public void CollectionAssertAreEqualWithUserMessage_FailsWithGoodMessage()
+    {
+        Exception ex = VerifyThrows(() => CollectionAssert.AreEqual(new[] { 1, 2, 3 }, new[] { 1, 5, 3 }, "User-provided message"));
+        Assert.AreEqual(
+            """
+            CollectionAssert.AreEqual failed. User-provided message. Element at index 1 do not match.
+            Expected: 2
+            Actual: 5
             """, ex.Message);
     }
 

Original file line number	Diff line number	Diff line change
`@@ -1158,8 +1158,7 @@ public static void AreEqual(ICollection? expected, ICollection? actual, [StringS`
`1158`	`1158`	`string reason = string.Empty;`
`1159`	`1159`	`if (!AreCollectionsEqual(expected, actual, new ObjectComparer(), ref reason))`
`1160`	`1160`	`{`
`1161`		`- string userMessage = Assert.BuildUserMessage(message, parameters);`
`1162`		`- string finalMessage = string.Format(CultureInfo.CurrentCulture, FrameworkMessages.CollectionEqualReason, userMessage, reason);`
	`1161`	`+ string finalMessage = ConstructFinalMessage(reason, message, parameters);`
`1163`	`1162`	`Assert.ThrowAssertFailed("CollectionAssert.AreEqual", finalMessage);`
`1164`	`1163`	`}`
`1165`	`1164`	`}`
`@@ -1243,8 +1242,7 @@ public static void AreNotEqual(ICollection? notExpected, ICollection? actual, [S`
`1243`	`1242`	`string reason = string.Empty;`
`1244`	`1243`	`if (AreCollectionsEqual(notExpected, actual, new ObjectComparer(), ref reason))`
`1245`	`1244`	`{`
`1246`		`- string userMessage = Assert.BuildUserMessage(message, parameters);`
`1247`		`- string finalMessage = string.Format(CultureInfo.CurrentCulture, FrameworkMessages.CollectionEqualReason, userMessage, reason);`
	`1245`	`+ string finalMessage = ConstructFinalMessage(reason, message, parameters);`
`1248`	`1246`	`Assert.ThrowAssertFailed("CollectionAssert.AreNotEqual", finalMessage);`
`1249`	`1247`	`}`
`1250`	`1248`	`}`
`@@ -1335,8 +1333,7 @@ public static void AreEqual(ICollection? expected, ICollection? actual, [NotNull`
`1335`	`1333`	`string reason = string.Empty;`
`1336`	`1334`	`if (!AreCollectionsEqual(expected, actual, comparer, ref reason))`
`1337`	`1335`	`{`
`1338`		`- string userMessage = Assert.BuildUserMessage(message, parameters);`
`1339`		`- string finalMessage = string.Format(CultureInfo.CurrentCulture, FrameworkMessages.CollectionEqualReason, userMessage, reason);`
	`1336`	`+ string finalMessage = ConstructFinalMessage(reason, message, parameters);`
`1340`	`1337`	`Assert.ThrowAssertFailed("CollectionAssert.AreEqual", finalMessage);`
`1341`	`1338`	`}`
`1342`	`1339`	`}`
`@@ -1427,8 +1424,7 @@ public static void AreNotEqual(ICollection? notExpected, ICollection? actual, [N`
`1427`	`1424`	`string reason = string.Empty;`
`1428`	`1425`	`if (AreCollectionsEqual(notExpected, actual, comparer, ref reason))`
`1429`	`1426`	`{`
`1430`		`- string userMessage = Assert.BuildUserMessage(message, parameters);`
`1431`		`- string finalMessage = string.Format(CultureInfo.CurrentCulture, FrameworkMessages.CollectionEqualReason, userMessage, reason);`
	`1427`	`+ string finalMessage = ConstructFinalMessage(reason, message, parameters);`
`1432`	`1428`	`Assert.ThrowAssertFailed("CollectionAssert.AreNotEqual", finalMessage);`
`1433`	`1429`	`}`
`1434`	`1430`	`}`
`@@ -1674,6 +1670,17 @@ private static bool CompareIEnumerable(IEnumerable? expected, IEnumerable? actua`
`1674`	`1670`	`return true;`
`1675`	`1671`	`}`
`1676`	`1672`
	`1673`	`+ private static string ConstructFinalMessage(`
	`1674`	`+ string reason,`
	`1675`	`+ [StringSyntax(StringSyntaxAttribute.CompositeFormat)] string? message,`
	`1676`	`+ params object?[]? parameters)`
	`1677`	`+ {`
	`1678`	`+ string userMessage = Assert.BuildUserMessage(message, parameters);`
	`1679`	`+ return userMessage.Length == 0`
	`1680`	`+ ? reason`
	`1681`	`+ : string.Format(CultureInfo.CurrentCulture, FrameworkMessages.CollectionEqualReason, userMessage, reason);`
	`1682`	`+ }`
	`1683`	`+`
`1677`	`1684`	`/// <summary>`
`1678`	`1685`	`/// compares the objects using object.Equals.`
`1679`	`1686`	`/// </summary>`