From 7a4394b00af74e878035016a70311ba3300094c2 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Thu, 25 May 2023 14:57:31 +0100 Subject: [PATCH 1/4] add tests for maps, including failing test for null values (issue 2913) --- .../java/test/asserttests/AssertTest.java | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/testng-asserts/src/test/java/test/asserttests/AssertTest.java b/testng-asserts/src/test/java/test/asserttests/AssertTest.java index 8eacebf6d..c8bc468c4 100644 --- a/testng-asserts/src/test/java/test/asserttests/AssertTest.java +++ b/testng-asserts/src/test/java/test/asserttests/AssertTest.java @@ -563,4 +563,92 @@ public void checkArrayNotEqualsFailsWhenDifferentOrder() { assertNotEquals(array1, array2); } + + @Test + public void checkMapEquals() { + Map map1 = new LinkedHashMap<>(); + map1.put("a", "1"); + map1.put("b", 2); + Map map2 = new LinkedHashMap<>(); + map2.put("b", 2); + map2.put("a", "1"); + + assertMapsEqual(map1, map2); + } + + @Test + public void checkMapNotEquals() { + Map map1 = new LinkedHashMap<>(); + map1.put("a", "1"); + map1.put("b", "2"); + Map map2 = new LinkedHashMap<>(); + map2.put("a", "1"); + map2.put("b", 2); + + assertMapsNotEqual(map1, map2); + } + + @Test(description = "GITHUB-2913") + public void checkMapNotEqualsWithNull() { + Map map1 = new LinkedHashMap<>(); + map1.put("a", 1); + map1.put("b", null); + Map map2 = new LinkedHashMap<>(); + map2.put("a", 1); + map2.put("c", null); + + assertMapsNotEqual(map1, map2); + } + + protected void assertMapsEqual(Map m1, Map m2) { + assertEquals((Object) m1, (Object) m2); + assertEquals(m1, m2); + + boolean succeeded = false; + try { + assertNotEquals((Object) m1, (Object) m2); + succeeded = true; + } catch (AssertionError e) { + /* expected */ + } + if (succeeded) { + Assert.fail("Maps reported as not equal when equal: " + m1 + " / " + m2); + } + + try { + assertNotEquals(m1, m2); + succeeded = true; + } catch (AssertionError e) { + /* expected */ + } + if (succeeded) { + Assert.fail("Maps reported as not equal when equal: " + m1 + " / " + m2); + } + } + + protected void assertMapsNotEqual(Map m1, Map m2) { + boolean succeeded = false; + try { + assertEquals((Object) m1, (Object) m2); + succeeded = true; + } catch (AssertionError e) { + /* expected */ + } + if (succeeded) { + Assert.fail("Maps reported as equal when not equal: " + m1 + " / " + m2); + } + + try { + assertEquals(m1, m2); + succeeded = true; + } catch (AssertionError e) { + /* expected */ + } + if (succeeded) { + Assert.fail("Maps reported as equal when not equal: " + m1 + " / " + m2); + } + + assertNotEquals((Object) m1, (Object) m2); + assertNotEquals(m1, m2); + } } From 4216a3d7a3d64e605a8a570c9e5dbaf7eea1b3db Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Thu, 25 May 2023 15:05:27 +0100 Subject: [PATCH 2/4] fix the failing test for maps with null values --- testng-asserts/src/main/java/org/testng/Assert.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/testng-asserts/src/main/java/org/testng/Assert.java b/testng-asserts/src/main/java/org/testng/Assert.java index 40ca2be66..996196c31 100644 --- a/testng-asserts/src/main/java/org/testng/Assert.java +++ b/testng-asserts/src/main/java/org/testng/Assert.java @@ -2109,11 +2109,14 @@ private static String getNotEqualReason(Map actual, Map expected) { Object key = entry.getKey(); Object value = entry.getValue(); Object expectedValue = expected.get(key); - String assertMessage = - "Maps do not match for key:" + key + " actual:" + value + " expected:" + expectedValue; if (!areEqualImpl(value, expectedValue)) { + String assertMessage = + "Maps do not match for key:" + key + " actual:" + value + " expected:" + expectedValue; return assertMessage; } + if (value == null && !expected.containsKey(key)) { + return "Maps do not match for key:" + key + " actual: null but not present in expected"; + } } return null; } From 732fb0d41b39b79740c36c222895d1917c985942 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Thu, 25 May 2023 15:08:13 +0100 Subject: [PATCH 3/4] update CHANGES.txt --- CHANGES.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index c21f84e5f..b4d79f37e 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,5 +1,7 @@ Current +Fixed: GITHUB-2913: Maps containing nulls can be incorrectly considered equal + 7.8.0 Fixed: GITHUB-2906: Generate testng-results.xml per test suite (Krishnan Mahadevan) New: GITHUB-2897: Not exception but warning if some (not all) of the given test names are not found in suite files. (Bruce Wen) From 88eb73fb60f9c64d2cbcc92fe95c02659bb09091 Mon Sep 17 00:00:00 2001 From: Alex Heneveld Date: Mon, 29 May 2023 17:44:27 +0100 Subject: [PATCH 4/4] apply code review comments - TY --- CHANGES.txt | 2 +- .../java/test/asserttests/AssertTest.java | 28 ++++++------------- 2 files changed, 9 insertions(+), 21 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index b4d79f37e..c92c2cf8b 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,6 +1,6 @@ Current -Fixed: GITHUB-2913: Maps containing nulls can be incorrectly considered equal +Fixed: GITHUB-2913: Maps containing nulls can be incorrectly considered equal (Alex Heneveld) 7.8.0 Fixed: GITHUB-2906: Generate testng-results.xml per test suite (Krishnan Mahadevan) diff --git a/testng-asserts/src/test/java/test/asserttests/AssertTest.java b/testng-asserts/src/test/java/test/asserttests/AssertTest.java index c8bc468c4..c500fa131 100644 --- a/testng-asserts/src/test/java/test/asserttests/AssertTest.java +++ b/testng-asserts/src/test/java/test/asserttests/AssertTest.java @@ -608,22 +608,16 @@ protected void assertMapsEqual(Map m1, Map m2) { try { assertNotEquals((Object) m1, (Object) m2); succeeded = true; - } catch (AssertionError e) { - /* expected */ - } - if (succeeded) { - Assert.fail("Maps reported as not equal when equal: " + m1 + " / " + m2); + } catch (AssertionError expected) { } + assertFalse(succeeded, "Maps reported as not equal when equal: " + m1 + " / " + m2); try { assertNotEquals(m1, m2); succeeded = true; - } catch (AssertionError e) { - /* expected */ - } - if (succeeded) { - Assert.fail("Maps reported as not equal when equal: " + m1 + " / " + m2); + } catch (AssertionError expected) { } + assertFalse(succeeded, "Maps reported as not equal when equal: " + m1 + " / " + m2); } protected void assertMapsNotEqual(Map m1, Map m2) { @@ -631,22 +625,16 @@ protected void assertMapsNotEqual(Map m1, Map m2) { try { assertEquals((Object) m1, (Object) m2); succeeded = true; - } catch (AssertionError e) { - /* expected */ - } - if (succeeded) { - Assert.fail("Maps reported as equal when not equal: " + m1 + " / " + m2); + } catch (AssertionError expected) { } + assertFalse(succeeded, "Maps reported as equal when not equal: " + m1 + " / " + m2); try { assertEquals(m1, m2); succeeded = true; - } catch (AssertionError e) { - /* expected */ - } - if (succeeded) { - Assert.fail("Maps reported as equal when not equal: " + m1 + " / " + m2); + } catch (AssertionError expected) { } + assertFalse(succeeded, "Maps reported as equal when not equal: " + m1 + " / " + m2); assertNotEquals((Object) m1, (Object) m2); assertNotEquals(m1, m2);