Skip to content
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

[Feature] Add custom description to assertThat and/or Locator naming #954

Open
shalak opened this issue Jun 16, 2022 · 1 comment
Open

Comments

@shalak
Copy link

shalak commented Jun 16, 2022

With assertJ, we can add a custom name for given asserted object, for example:

Assertions.assertThat(mansion.guests()).as("Living Guests").isEqualTo(7);

Will result in:

[Living Guests] expected:<[7]> but was<[6]>

Selenide comes with element-aliasing as well as the .because() call (that does the similar thing, but not on asserted element, but the assertion itself):

$("div >> span").as("Living Guests").shouldHaveSize(7).because("The guest count should be valid")

The playwright-java assertions would be more readable if we'd be able to do similar thing with PlaywrightAssertions.assertThat or even the Locator/Page/ApiResponse itself, e.g.:

// on assertion:
PlaywrightAssertions.assertThat(questCount).hasText("7").because("The guest count should be valid");
// on asserted object:
PlaywrightAssertions.assertThat(questCount).as("Living Guests").hasText("7");
// on Locator
Locator guestCount = page.locator("div >> span").as("Living Guests");
@shalak
Copy link
Author

shalak commented Jun 17, 2022

Quick PoC on the Locator & Page, as this looks to be the simplest to implement:

diff --git a/playwright/src/main/java/com/microsoft/playwright/Aliasable.java b/playwright/src/main/java/com/microsoft/playwright/Aliasable.java
new file mode 100644
index 0000000..8d03faf
--- /dev/null
+++ b/playwright/src/main/java/com/microsoft/playwright/Aliasable.java
@@ -0,0 +1,6 @@
+package com.microsoft.playwright;
+
+public interface Aliasable<T> {
+  String getAlias();
+  T setAlias(String name);
+}
diff --git a/playwright/src/main/java/com/microsoft/playwright/Locator.java b/playwright/src/main/java/com/microsoft/playwright/Locator.java
index 667a879..7c5446d 100644
--- a/playwright/src/main/java/com/microsoft/playwright/Locator.java
+++ b/playwright/src/main/java/com/microsoft/playwright/Locator.java
@@ -28,7 +28,7 @@ import java.util.regex.Pattern;
  *
  * <p> <a href="https://playwright.dev/java/docs/locators">Learn more about locators</a>.
  */
-public interface Locator {
+public interface Locator extends Aliasable<Locator> {
   class BoundingBoxOptions {
     /**
      * Maximum time in milliseconds, defaults to 30 seconds, pass {@code 0} to disable timeout. The default value can be changed by
diff --git a/playwright/src/main/java/com/microsoft/playwright/Page.java b/playwright/src/main/java/com/microsoft/playwright/Page.java
index 1d6ee25..042e25c 100644
--- a/playwright/src/main/java/com/microsoft/playwright/Page.java
+++ b/playwright/src/main/java/com/microsoft/playwright/Page.java
@@ -66,7 +66,7 @@ import java.util.regex.Pattern;
  * page.offRequest(logRequest);
  * }</pre>
  */
-public interface Page extends AutoCloseable {
+public interface Page extends AutoCloseable, Aliasable<Page> {
 
   /**
    * Emitted when the page closes.
diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/LocatorAssertionsImpl.java b/playwright/src/main/java/com/microsoft/playwright/impl/LocatorAssertionsImpl.java
index 246e536..3801229 100644
--- a/playwright/src/main/java/com/microsoft/playwright/impl/LocatorAssertionsImpl.java
+++ b/playwright/src/main/java/com/microsoft/playwright/impl/LocatorAssertionsImpl.java
@@ -41,7 +41,7 @@ public class LocatorAssertionsImpl extends AssertionsBase implements LocatorAsse
     expected.string = text;
     expected.matchSubstring = true;
     expected.normalizeWhiteSpace = true;
-    expectImpl("to.have.text", expected, text, "Locator expected to contain text", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.have.text", expected, text, actualLocator.getAlias() + " expected to contain text", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
@@ -49,7 +49,7 @@ public class LocatorAssertionsImpl extends AssertionsBase implements LocatorAsse
     ExpectedTextValue expected = expectedRegex(pattern);
     expected.matchSubstring = true;
     expected.normalizeWhiteSpace = true;
-    expectImpl("to.have.text", expected, pattern, "Locator expected to contain regex", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.have.text", expected, pattern, actualLocator.getAlias() + " expected to contain regex", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
@@ -62,7 +62,7 @@ public class LocatorAssertionsImpl extends AssertionsBase implements LocatorAsse
       expected.normalizeWhiteSpace = true;
       list.add(expected);
     }
-    expectImpl("to.contain.text.array", list, strings, "Locator expected to contain text", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.contain.text.array", list, strings, actualLocator.getAlias() + " expected to contain text", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
@@ -74,7 +74,7 @@ public class LocatorAssertionsImpl extends AssertionsBase implements LocatorAsse
       expected.normalizeWhiteSpace = true;
       list.add(expected);
     }
-    expectImpl("to.contain.text.array", list, patterns, "Locator expected to contain text", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.contain.text.array", list, patterns, actualLocator.getAlias() + " expected to contain text", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
@@ -96,7 +96,7 @@ public class LocatorAssertionsImpl extends AssertionsBase implements LocatorAsse
     }
     FrameExpectOptions commonOptions = convertType(options, FrameExpectOptions.class);
     commonOptions.expressionArg = name;
-    String message = "Locator expected to have attribute '" + name + "'";
+    String message = actualLocator.getAlias() + " expected to have attribute '" + name + "'";
     if (expectedValue instanceof Pattern) {
       message += " matching regex";
     }
@@ -107,13 +107,13 @@ public class LocatorAssertionsImpl extends AssertionsBase implements LocatorAsse
   public void hasClass(String text, HasClassOptions options) {
     ExpectedTextValue expected = new ExpectedTextValue();
     expected.string = text;
-    expectImpl("to.have.class", expected, text, "Locator expected to have class", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.have.class", expected, text, actualLocator.getAlias() + " expected to have class", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
   public void hasClass(Pattern pattern, HasClassOptions options) {
     ExpectedTextValue expected = expectedRegex(pattern);
-    expectImpl("to.have.class", expected, pattern, "Locator expected to have class matching regex", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.have.class", expected, pattern, actualLocator.getAlias() + " expected to have class matching regex", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
@@ -124,7 +124,7 @@ public class LocatorAssertionsImpl extends AssertionsBase implements LocatorAsse
       expected.string = text;
       list.add(expected);
     }
-    expectImpl("to.have.class.array", list, strings, "Locator expected to have class", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.have.class.array", list, strings, actualLocator.getAlias() + " expected to have class", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
@@ -134,7 +134,7 @@ public class LocatorAssertionsImpl extends AssertionsBase implements LocatorAsse
       ExpectedTextValue expected = expectedRegex(pattern);
       list.add(expected);
     }
-    expectImpl("to.have.class.array", list, patterns, "Locator expected to have class matching regex", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.have.class.array", list, patterns, actualLocator.getAlias() + " expected to have class matching regex", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
@@ -145,7 +145,7 @@ public class LocatorAssertionsImpl extends AssertionsBase implements LocatorAsse
     FrameExpectOptions commonOptions = convertType(options, FrameExpectOptions.class);
     commonOptions.expectedNumber = count;
     List<ExpectedTextValue> expectedText = null;
-    expectImpl("to.have.count", expectedText, count, "Locator expected to have count", commonOptions);
+    expectImpl("to.have.count", expectedText, count, actualLocator.getAlias() + " expected to have count", commonOptions);
   }
 
   @Override
@@ -167,7 +167,7 @@ public class LocatorAssertionsImpl extends AssertionsBase implements LocatorAsse
     }
     FrameExpectOptions commonOptions = convertType(options, FrameExpectOptions.class);
     commonOptions.expressionArg = name;
-    String message = "Locator expected to have CSS property '" + name + "'";
+    String message = actualLocator.getAlias() + " expected to have CSS property '" + name + "'";
     if (expectedValue instanceof Pattern) {
       message += " matching regex";
     }
@@ -178,13 +178,13 @@ public class LocatorAssertionsImpl extends AssertionsBase implements LocatorAsse
   public void hasId(String id, HasIdOptions options) {
     ExpectedTextValue expected = new ExpectedTextValue();
     expected.string = id;
-    expectImpl("to.have.id", expected, id, "Locator expected to have ID", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.have.id", expected, id, actualLocator.getAlias() + " expected to have ID", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
   public void hasId(Pattern pattern, HasIdOptions options) {
     ExpectedTextValue expected = expectedRegex(pattern);
-    expectImpl("to.have.id", expected, pattern, "Locator expected to have ID matching regex", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.have.id", expected, pattern, actualLocator.getAlias() + " expected to have ID matching regex", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
@@ -196,7 +196,7 @@ public class LocatorAssertionsImpl extends AssertionsBase implements LocatorAsse
     commonOptions.expressionArg = name;
     commonOptions.expectedValue = serializeArgument(value);
     List<ExpectedTextValue> list = null;
-    expectImpl("to.have.property", list, value, "Locator expected to have JavaScript property '" + name + "'", commonOptions);
+    expectImpl("to.have.property", list, value, actualLocator.getAlias() + " expected to have JavaScript property '" + name + "'", commonOptions);
   }
 
   @Override
@@ -205,7 +205,7 @@ public class LocatorAssertionsImpl extends AssertionsBase implements LocatorAsse
     expected.string = text;
     expected.matchSubstring = false;
     expected.normalizeWhiteSpace = true;
-    expectImpl("to.have.text", expected, text, "Locator expected to have text", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.have.text", expected, text, actualLocator.getAlias() + " expected to have text", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
@@ -214,7 +214,7 @@ public class LocatorAssertionsImpl extends AssertionsBase implements LocatorAsse
     // Just match substring, same as containsText.
     expected.matchSubstring = true;
     expected.normalizeWhiteSpace = true;
-    expectImpl("to.have.text", expected, pattern, "Locator expected to have text matching regex", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.have.text", expected, pattern, actualLocator.getAlias() + " expected to have text matching regex", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
@@ -227,7 +227,7 @@ public class LocatorAssertionsImpl extends AssertionsBase implements LocatorAsse
       expected.normalizeWhiteSpace = true;
       list.add(expected);
     }
-    expectImpl("to.have.text.array", list, strings, "Locator expected to have text", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.have.text.array", list, strings, actualLocator.getAlias() + " expected to have text", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
@@ -239,61 +239,61 @@ public class LocatorAssertionsImpl extends AssertionsBase implements LocatorAsse
       expected.normalizeWhiteSpace = true;
       list.add(expected);
     }
-    expectImpl("to.have.text.array", list, patterns, "Locator expected to have text matching regex", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.have.text.array", list, patterns, actualLocator.getAlias() + " expected to have text matching regex", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
   public void hasValue(String value, HasValueOptions options) {
     ExpectedTextValue expected = new ExpectedTextValue();
     expected.string = value;
-    expectImpl("to.have.value", expected, value, "Locator expected to have value", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.have.value", expected, value, actualLocator.getAlias() + " expected to have value", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
   public void hasValue(Pattern pattern, HasValueOptions options) {
     ExpectedTextValue expected = expectedRegex(pattern);
-    expectImpl("to.have.value", expected, pattern, "Locator expected to have value matching regex", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.have.value", expected, pattern, actualLocator.getAlias() + " expected to have value matching regex", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
   public void isChecked(IsCheckedOptions options) {
     String expression = (options != null && options.checked != null && !options.checked) ? "to.be.unchecked" : "to.be.checked";
-    expectTrue(expression, "Locator expected to be checked", convertType(options, FrameExpectOptions.class));
+    expectTrue(expression, actualLocator.getAlias() + " expected to be checked", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
   public void isDisabled(IsDisabledOptions options) {
-    expectTrue("to.be.disabled", "Locator expected to be disabled", convertType(options, FrameExpectOptions.class));
+    expectTrue("to.be.disabled", actualLocator.getAlias() + " expected to be disabled", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
   public void isEditable(IsEditableOptions options) {
-    expectTrue("to.be.editable", "Locator expected to be editable", convertType(options, FrameExpectOptions.class));
+    expectTrue("to.be.editable", actualLocator.getAlias() + " expected to be editable", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
   public void isEmpty(IsEmptyOptions options) {
-    expectTrue("to.be.empty", "Locator expected to be empty", convertType(options, FrameExpectOptions.class));
+    expectTrue("to.be.empty", actualLocator.getAlias() + " expected to be empty", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
   public void isEnabled(IsEnabledOptions options) {
-    expectTrue("to.be.enabled", "Locator expected to be enabled", convertType(options, FrameExpectOptions.class));
+    expectTrue("to.be.enabled", actualLocator.getAlias() + " expected to be enabled", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
   public void isFocused(IsFocusedOptions options) {
-    expectTrue("to.be.focused", "Locator expected to be focused", convertType(options, FrameExpectOptions.class));
+    expectTrue("to.be.focused", actualLocator.getAlias() + " expected to be focused", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
   public void isHidden(IsHiddenOptions options) {
-    expectTrue("to.be.hidden", "Locator expected to be hidden", convertType(options, FrameExpectOptions.class));
+    expectTrue("to.be.hidden", actualLocator.getAlias() + " expected to be hidden", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
   public void isVisible(IsVisibleOptions options) {
-    expectTrue("to.be.visible", "Locator expected to be visible", convertType(options, FrameExpectOptions.class));
+    expectTrue("to.be.visible", actualLocator.getAlias() + " expected to be visible", convertType(options, FrameExpectOptions.class));
   }
 
   private void expectTrue(String expression, String message, FrameExpectOptions options) {
diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/LocatorImpl.java b/playwright/src/main/java/com/microsoft/playwright/impl/LocatorImpl.java
index 1e8a2be..91b0141 100644
--- a/playwright/src/main/java/com/microsoft/playwright/impl/LocatorImpl.java
+++ b/playwright/src/main/java/com/microsoft/playwright/impl/LocatorImpl.java
@@ -23,6 +23,18 @@ import static com.microsoft.playwright.impl.Utils.toJsRegexFlags;
 class LocatorImpl implements Locator {
   private final FrameImpl frame;
   private final String selector;
+  private String alias;
+
+  @Override
+  public String getAlias() {
+    return alias != null ? alias : "Locator";
+  }
+
+  @Override
+  public Locator setAlias(String name) {
+    this.alias = name;
+    return this;
+  }
 
   private static class Filters {
     private final Map<Field, String> filterFieldToEngine = new LinkedHashMap<>();
diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/PageAssertionsImpl.java b/playwright/src/main/java/com/microsoft/playwright/impl/PageAssertionsImpl.java
index ee6e715..57ea490 100644
--- a/playwright/src/main/java/com/microsoft/playwright/impl/PageAssertionsImpl.java
+++ b/playwright/src/main/java/com/microsoft/playwright/impl/PageAssertionsImpl.java
@@ -41,13 +41,13 @@ public class PageAssertionsImpl extends AssertionsBase implements PageAssertions
     ExpectedTextValue expected = new ExpectedTextValue();
     expected.string = title;
     expected.normalizeWhiteSpace = true;
-    expectImpl("to.have.title", expected, title, "Page title expected to be", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.have.title", expected, title, actualPage.getAlias() + " title expected to be", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
   public void hasTitle(Pattern pattern, HasTitleOptions options) {
     ExpectedTextValue expected = expectedRegex(pattern);
-    expectImpl("to.have.title", expected, pattern, "Page title expected to match regex", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.have.title", expected, pattern, actualPage.getAlias() + " title expected to match regex", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
@@ -57,13 +57,13 @@ public class PageAssertionsImpl extends AssertionsBase implements PageAssertions
       url = resolveUrl(actualPage.context().baseUrl, url);
     }
     expected.string = url;
-    expectImpl("to.have.url", expected, url, "Page URL expected to be", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.have.url", expected, url, actualPage.getAlias() + " URL expected to be", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
   public void hasURL(Pattern pattern, HasURLOptions options) {
     ExpectedTextValue expected = expectedRegex(pattern);
-    expectImpl("to.have.url", expected, pattern, "Page URL expected to match regex", convertType(options, FrameExpectOptions.class));
+    expectImpl("to.have.url", expected, pattern, actualPage.getAlias() + " URL expected to match regex", convertType(options, FrameExpectOptions.class));
   }
 
   @Override
diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/PageImpl.java b/playwright/src/main/java/com/microsoft/playwright/impl/PageImpl.java
index 84bb3a2..697efe9 100644
--- a/playwright/src/main/java/com/microsoft/playwright/impl/PageImpl.java
+++ b/playwright/src/main/java/com/microsoft/playwright/impl/PageImpl.java
@@ -48,6 +48,18 @@ public class PageImpl extends ChannelOwner implements Page {
   private ViewportSize viewport;
   private final Router routes = new Router();
   private final Set<FrameImpl> frames = new LinkedHashSet<>();
+  private String alias;
+
+  @Override
+  public String getAlias() {
+    return alias != null ? alias : "Page";
+  }
+
+  public Page setAlias(String name) {
+    this.alias = name;
+    return this;
+  }
+
   final ListenerCollection<EventType> listeners = new ListenerCollection<EventType>() {
     @Override
     void add(EventType eventType, Consumer<?> listener) {

Simple class to show of the messages:

public class AliasesPresentationTest {

    @Test
    void pageAliased() {
        try (Playwright playwright = Playwright.create()) {
            Browser browser = playwright.chromium().launch();
            Page page = browser.newPage().setAlias("Playwright page");
            page.navigate("http://playwright.dev");
            assertThat(page).hasTitle("This is not the expected result");
        }
    }

    @Test
    void locatorAliased() {
        try (Playwright playwright = Playwright.create()) {
            Browser browser = playwright.chromium().launch();
            Page page = browser.newPage().setAlias("Playwright page");
            page.navigate("http://playwright.dev");
            var allSpans = page.locator("span").setAlias("span element");
            assertThat(allSpans).hasCount(1);
        }
    }
    
}

Will result in:

org.opentest4j.AssertionFailedError: span element expected to have count: 1
Received: 9

...

org.opentest4j.AssertionFailedError: Playwright page title expected to be: This is not the expected result
Received: Fast and reliable end-to-end testing for modern web apps | Playwright

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants