diff --git a/README.md b/README.md index 2cdb2bb..e59f4a2 100644 --- a/README.md +++ b/README.md @@ -11,8 +11,10 @@ evolve and be released independently. * JDK 11 (or higher) -The assertion classes themselves depend only on the JDK. TestNG is required at **test scope** only -(to run the assertion tests via the TestNG runner). +`org.testng.Assert` delegates the behaviour-identical assertions (`assertNull`/`assertNotNull`/ +`assertSame`/`assertNotSame`, `assertThrows`) to [AssertJ](https://assertj.github.io/doc/), so +`assertj-core` is the only runtime dependency (brought transitively). TestNG is required at **test +scope** only (to run the assertion tests via the TestNG runner). ## Usage @@ -31,6 +33,8 @@ If you previously relied on `org.testng.Assert` / `org.testng.asserts.*` coming `org.testng:testng`, add `org.testng:testng-asserts` explicitly. For new code or larger refactoring efforts, [AssertJ](https://assertj.github.io/doc/) is recommended. +See [docs/MIGRATING_ASSERTIONS.md](docs/MIGRATING_ASSERTIONS.md) for the migration guide, including +the automated OpenRewrite recipe. ## Building diff --git a/docs/MIGRATING_ASSERTIONS.md b/docs/MIGRATING_ASSERTIONS.md new file mode 100644 index 0000000..326fc7b --- /dev/null +++ b/docs/MIGRATING_ASSERTIONS.md @@ -0,0 +1,158 @@ +# Migrating away from `org.testng.Assert` + +`org.testng.Assert` is **deprecated**. TestNG no longer wants to maintain its own assertion library +and instead recommends a dedicated one such as [AssertJ](https://assertj.github.io/doc/). + +This is part of a longer plan: + +- [testng-asserts#15](https://github.com/testng-team/testng-asserts/issues/15) — deprecate the + internal assertion API and let users choose their preferred library. +- [testng#3197](https://github.com/testng-team/testng/issues/3197) — make the `testng-asserts` + module optional. + +`org.testng.Assert` now lives in the standalone `org.testng:testng-asserts` artifact, separate from +the main `org.testng:testng` jar. Migrating now keeps your test suite working across that transition. + +## What replaces `org.testng.Assert`? + +[AssertJ](https://assertj.github.io/doc/). Its fluent API covers everything `org.testng.Assert` +offers, with better failure messages: + +```java +// Before — org.testng.Assert +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; + +assertEquals(actual, expected); +assertTrue(result); + +// After — AssertJ +import static org.assertj.core.api.Assertions.assertThat; + +assertThat(actual).isEqualTo(expected); +assertThat(result).isTrue(); +``` + +> **Note the parameter order.** `org.testng.Assert` uses `(actual, expected)`. AssertJ reads +> `assertThat(actual).isEqualTo(expected)`. The automated recipe below handles this for you. + +Add AssertJ to your build: + +```xml + + + org.assertj + assertj-core + test + +``` + +```kotlin +// Gradle +testImplementation("org.assertj:assertj-core:") +``` + +## Automated migration with OpenRewrite + +[OpenRewrite](https://docs.openrewrite.org/) provides a recipe that rewrites **the entire** +`org.testng.Assert` API to AssertJ. The full Assert coverage is available since +[rewrite-testing-frameworks v3.39.0](https://github.com/openrewrite/rewrite-testing-frameworks/releases/tag/v3.39.0). + +Recipe: `org.openrewrite.java.testing.testng.TestNgToAssertj` — *"Migrate TestNG assertions to +AssertJ"*. It handles both qualified (`Assert.assertEquals(...)`) and static-import +(`assertEquals(...)`) usages. + +### Gradle + +Apply the OpenRewrite plugin and activate the recipe: + +```kotlin +plugins { + id("org.openrewrite.rewrite") version "" +} + +dependencies { + rewrite("org.openrewrite.recipe:rewrite-testing-frameworks:<3.39.0-or-newer>") +} + +rewrite { + activeRecipe("org.openrewrite.java.testing.testng.TestNgToAssertj") +} +``` + +Run it: + +```bash +./gradlew rewriteRun +``` + +### Maven + +```xml + + org.openrewrite.maven + rewrite-maven-plugin + + + + org.openrewrite.java.testing.testng.TestNgToAssertj + + + + + org.openrewrite.recipe + rewrite-testing-frameworks + + + + +``` + +Run it: + +```bash +mvn rewrite:run +``` + +> Check the [OpenRewrite docs](https://docs.openrewrite.org/) for the current plugin and recipe +> versions. After the recipe runs, add the `org.assertj:assertj-core` test dependency (shown above) +> and run your test suite to confirm everything passes. + +## Manual migration + +If you prefer to migrate by hand, the most common mappings are: + +| `org.testng.Assert` | AssertJ | +| --------------------------------------- | ------------------------------------------------ | +| `assertEquals(actual, expected)` | `assertThat(actual).isEqualTo(expected)` | +| `assertNotEquals(actual, expected)` | `assertThat(actual).isNotEqualTo(expected)` | +| `assertTrue(condition)` | `assertThat(condition).isTrue()` | +| `assertFalse(condition)` | `assertThat(condition).isFalse()` | +| `assertNull(object)` | `assertThat(object).isNull()` | +| `assertNotNull(object)` | `assertThat(object).isNotNull()` | +| `assertSame(actual, expected)` | `assertThat(actual).isSameAs(expected)` | +| `assertNotSame(actual, expected)` | `assertThat(actual).isNotSameAs(expected)` | +| `assertThrows(Type.class, runnable)` | `assertThatThrownBy(runnable).isInstanceOf(Type.class)` | +| `fail(message)` | `fail(message)` (`org.assertj.core.api.Assertions.fail`) | + +See the [AssertJ documentation](https://assertj.github.io/doc/) for the full API. + +## Packaging notes + +`org.testng.Assert` is no longer bundled in the main `org.testng:testng` jar. To keep using it, add +the standalone artifact, which brings AssertJ transitively: + +```xml + + org.testng + testng-asserts + test + +``` + +> **OSGi:** `testng-asserts` currently ships as a plain JAR, not an OSGi bundle. `org.testng.Assert` +> and `org.testng.FileAssert` share the `org.testng` package with the core classes exported by the +> `testng` bundle, so giving `testng-asserts` its own bundle would create a split package. Proper +> OSGi support is tracked as a follow-up +> ([testng#3197](https://github.com/testng-team/testng/issues/3197)); OSGi users should prefer +> migrating to AssertJ. diff --git a/pom.xml b/pom.xml index a792cea..1182bce 100644 --- a/pom.xml +++ b/pom.xml @@ -63,20 +63,20 @@ - - org.testng - testng - [7.10.2,) - test - org.assertj assertj-core 3.27.7 + + + org.testng + testng + [7.10.2,) test diff --git a/src/main/java/org/testng/Assert.java b/src/main/java/org/testng/Assert.java index 11370c9..5fb55b4 100644 --- a/src/main/java/org/testng/Assert.java +++ b/src/main/java/org/testng/Assert.java @@ -1,7 +1,8 @@ package org.testng; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.testng.internal.EclipseInterface.ASSERT_EQUAL_LEFT; -import static org.testng.internal.EclipseInterface.ASSERT_LEFT2; import static org.testng.internal.EclipseInterface.ASSERT_MIDDLE; import static org.testng.internal.EclipseInterface.ASSERT_RIGHT; import static org.testng.internal.EclipseInterface.ASSERT_UNEQUAL_LEFT; @@ -34,7 +35,14 @@ * assertFalse(var.equals(null))}. * * @author Alexandru Popescu + * @deprecated TestNG no longer maintains its own assertion library. Use a dedicated assertion + * library such as AssertJ instead. The OpenRewrite + * recipe {@code org.openrewrite.java.testing.testng.TestNgToAssertj} migrates the whole class + * automatically; see the + * migration guide for details. */ +@Deprecated public class Assert { public static final String ARRAY_MISMATCH_TEMPLATE = @@ -53,9 +61,8 @@ protected Assert() { * @param message the assertion error message */ public static void assertTrue(boolean condition, String message) { - if (!condition) { - failNotEquals(condition, Boolean.TRUE, message); - } + // Delegated to AssertJ (behaviour-identical, only the failure message differs). + assertThat(condition).as(message).isTrue(); } /** @@ -75,9 +82,8 @@ public static void assertTrue(boolean condition) { * @param message the assertion error message */ public static void assertFalse(boolean condition, String message) { - if (condition) { - failNotEquals(condition, Boolean.FALSE, message); // TESTNG-81 - } + // Delegated to AssertJ (behaviour-identical, only the failure message differs). + assertThat(condition).as(message).isFalse(); } /** @@ -96,10 +102,9 @@ public static void assertFalse(boolean condition) { * @param realCause the original exception */ public static void fail(String message, Throwable realCause) { - AssertionError ae = new AssertionError(message); - ae.initCause(realCause); - - throw ae; + // Delegated to AssertJ (behaviour-identical: throws an AssertionError with the given message + // and cause). + org.assertj.core.api.Assertions.fail(message, realCause); } /** @@ -108,7 +113,8 @@ public static void fail(String message, Throwable realCause) { * @param message the assertion error message */ public static void fail(String message) { - throw new AssertionError(message); + // Delegated to AssertJ (behaviour-identical: throws an AssertionError with the given message). + org.assertj.core.api.Assertions.fail(message); } /** Fails a test with no message. */ @@ -169,6 +175,17 @@ private static boolean areNotEqualImpl(Object actual, Object expected) { return !expected.equals(actual); } + // NOTE: equality comparison is intentionally NOT delegated to AssertJ; it keeps TestNG's own + // semantics, which differ from AssertJ on purpose: + // - equality must be symmetric: both expected.equals(actual) and actual.equals(expected) must + // hold (AssertJ only checks one direction); + // - a broken equals() returning true for null does not make the values equal here, whereas + // AssertJ trusts equals(); + // - arrays nested inside collections/maps/iterables are compared by reference (use + // assertEqualsDeep for value comparison), whereas AssertJ compares them deeply. + // These behaviours are covered by regression tests (e.g. GITHUB-2483, GITHUB-2490, GITHUB-2500, + // ArrayEqualityAssertTest); only behaviour-identical assertions (assertNull/NotNull/Same/NotSame) + // are delegated to AssertJ. private static boolean areEqualImpl(Object actual, Object expected) { if ((expected == null) && (actual == null)) { return true; @@ -1488,13 +1505,8 @@ public static void assertNotNull(Object object) { * @param message the assertion error message */ public static void assertNotNull(Object object, String message) { - if (object == null) { - String formatted = ""; - if (message != null) { - formatted = message + " "; - } - fail(formatted + "expected object to not be null"); - } + // Delegated to AssertJ (behaviour-identical, only the failure message differs). + assertThat(object).as(message).isNotNull(); } /** @@ -1515,9 +1527,8 @@ public static void assertNull(Object object) { * @param message the assertion error message */ public static void assertNull(Object object, String message) { - if (object != null) { - failNotSame(object, null, message); - } + // Delegated to AssertJ (behaviour-identical, only the failure message differs). + assertThat(object).as(message).isNull(); } /** @@ -1529,10 +1540,8 @@ public static void assertNull(Object object, String message) { * @param message the assertion error message */ public static void assertSame(Object actual, Object expected, String message) { - if (expected == actual) { - return; - } - failNotSame(actual, expected, message); + // Delegated to AssertJ (behaviour-identical, only the failure message differs). + assertThat(actual).as(message).isSameAs(expected); } /** @@ -1554,9 +1563,8 @@ public static void assertSame(Object actual, Object expected) { * @param message the assertion error message */ public static void assertNotSame(Object actual, Object expected, String message) { - if (expected == actual) { - failSame(actual, expected, message); - } + // Delegated to AssertJ (behaviour-identical, only the failure message differs). + assertThat(actual).as(message).isNotSameAs(expected); } /** @@ -1570,22 +1578,6 @@ public static void assertNotSame(Object actual, Object expected) { assertNotSame(actual, expected, null); } - private static void failSame(Object actual, Object expected, String message) { - String formatted = ""; - if (message != null) { - formatted = message + " "; - } - fail(formatted + ASSERT_LEFT2 + expected + ASSERT_MIDDLE + actual + ASSERT_RIGHT); - } - - private static void failNotSame(Object actual, Object expected, String message) { - String formatted = ""; - if (message != null) { - formatted = message + " "; - } - fail(formatted + ASSERT_EQUAL_LEFT + expected + ASSERT_MIDDLE + actual + ASSERT_RIGHT); - } - private static void failNotEquals(Object actual, Object expected, String message) { fail(format(actual, expected, message, true)); } @@ -2386,25 +2378,27 @@ public static void assertThrows(ThrowingRunnable runnable) { /** * Asserts that {@code runnable} throws an exception of type {@code throwableClass} when executed. * If it does not throw an exception, an {@link AssertionError} is thrown. If it throws the wrong - * type of exception, an {@code AssertionError} is thrown describing the mismatch; the exception - * that was actually thrown can be obtained by calling {@link AssertionError#getCause}. + * type of exception, an {@code AssertionError} is thrown describing the mismatch. To obtain the + * exception that was actually thrown, use {@link #expectThrows} instead. * * @param throwableClass the expected type of the exception * @param the expected type of the exception * @param runnable A function that is expected to throw an exception when invoked * @since 6.9.5 */ - @SuppressWarnings("ThrowableResultOfMethodCallIgnored") public static void assertThrows( Class throwableClass, ThrowingRunnable runnable) { - expectThrows(throwableClass, runnable); + // Delegated to AssertJ; the thrown-type check keeps TestNG semantics (subtypes accepted), only + // the failure message differs. expectThrows is kept as-is because it returns the caught + // exception, which AssertJ's fluent API does not expose. + assertThatExceptionOfType(throwableClass).isThrownBy(runnable::run); } /** * Asserts that {@code runnable} throws an exception of type {@code throwableClass} when executed. * If it does not throw an exception, an {@link AssertionError} is thrown. If it throws the wrong - * type of exception, an {@code AssertionError} is thrown describing the mismatch; the exception - * that was actually thrown can be obtained by calling {@link AssertionError#getCause}. + * type of exception, an {@code AssertionError} is thrown describing the mismatch. To obtain the + * exception that was actually thrown, use {@link #expectThrows} instead. * * @param message fail message * @param throwableClass the expected type of the exception @@ -2413,7 +2407,8 @@ public static void assertThrows( */ public static void assertThrows( String message, Class throwableClass, ThrowingRunnable runnable) { - expectThrows(message, throwableClass, runnable); + // Delegated to AssertJ (see the typed overload above). + assertThatExceptionOfType(throwableClass).as(message).isThrownBy(runnable::run); } /** diff --git a/src/main/java/org/testng/asserts/Assertion.java b/src/main/java/org/testng/asserts/Assertion.java index a3778de..bd20747 100644 --- a/src/main/java/org/testng/asserts/Assertion.java +++ b/src/main/java/org/testng/asserts/Assertion.java @@ -5,6 +5,7 @@ import java.util.Set; /** An assert class with various hooks allowing its behavior to be modified by subclasses. */ +@SuppressWarnings("deprecation") // delegates to the deprecated org.testng.Assert public class Assertion implements IAssertLifecycle { protected void doAssert(IAssert assertCommand) { onBeforeAssert(assertCommand); diff --git a/src/test/java/org/testng/AssertTest.java b/src/test/java/org/testng/AssertTest.java index 0f1a60a..65f0abf 100644 --- a/src/test/java/org/testng/AssertTest.java +++ b/src/test/java/org/testng/AssertTest.java @@ -437,7 +437,7 @@ public void testAssertEqualsNoOrderNotDeep() { @Test( description = "GITHUB-2080", expectedExceptions = AssertionError.class, - expectedExceptionsMessageRegExp = "test expected \\[true\\] but found \\[false\\]") + expectedExceptionsMessageRegExp = "(?s)\\[test\\] .*Expecting value to be true but was false") public void testAssertTrueMessage() { Assert.assertTrue(false, "test"); } @@ -445,7 +445,7 @@ public void testAssertTrueMessage() { @Test( description = "GITHUB-2080", expectedExceptions = AssertionError.class, - expectedExceptionsMessageRegExp = "test expected \\[false\\] but found \\[true\\]") + expectedExceptionsMessageRegExp = "(?s)\\[test\\] .*Expecting value to be false but was true") public void testAssertFalseMessage() { Assert.assertFalse(true, "test"); } diff --git a/src/test/java/test/assertion/AssertionTest.java b/src/test/java/test/assertion/AssertionTest.java index 25825b3..c170992 100644 --- a/src/test/java/test/assertion/AssertionTest.java +++ b/src/test/java/test/assertion/AssertionTest.java @@ -36,7 +36,9 @@ public void test2() { "onAfterAssert"); } - @Test(expectedExceptions = AssertionError.class, expectedExceptionsMessageRegExp = "Raw test .*") + @Test( + expectedExceptions = AssertionError.class, + expectedExceptionsMessageRegExp = "(?s)\\[Raw test\\] .*") public void test2_fails() { try { rawAssertion.assertTrue(true); diff --git a/src/test/java/test/assertion/SoftAssertTest.java b/src/test/java/test/assertion/SoftAssertTest.java index 23a4b91..a0b1b26 100644 --- a/src/test/java/test/assertion/SoftAssertTest.java +++ b/src/test/java/test/assertion/SoftAssertTest.java @@ -63,10 +63,11 @@ public void testAssertAllCount() { sa.assertAll(); Assert.fail("Exception expected"); } catch (AssertionError e) { - String[] lines = e.getMessage().split("\r?\n"); - Assert.assertEquals(lines.length, 2); - lines[1] = lines[1].replaceFirst(message, ""); - Assert.assertFalse(lines[1].contains(message)); + // Exactly one assertion failed, so the user message is reported exactly once. (Line counting + // is avoided here because the delegated AssertJ failure messages span multiple lines.) + String report = e.getMessage(); + Assert.assertTrue(report.contains(message), report); + Assert.assertEquals(report.indexOf(message), report.lastIndexOf(message), report); } }