-
Notifications
You must be signed in to change notification settings - Fork 233
chore: roll 1.53.0 #1801
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
chore: roll 1.53.0 #1801
Conversation
playwright/src/main/java/com/microsoft/playwright/impl/BrowserImpl.java
Outdated
Show resolved
Hide resolved
playwright/src/main/java/com/microsoft/playwright/impl/BrowserImpl.java
Outdated
Show resolved
Hide resolved
playwright/src/main/java/com/microsoft/playwright/impl/BrowserImpl.java
Outdated
Show resolved
Hide resolved
playwright/src/main/java/com/microsoft/playwright/impl/BrowserImpl.java
Outdated
Show resolved
Hide resolved
playwright/src/main/java/com/microsoft/playwright/impl/BrowserTypeImpl.java
Show resolved
Hide resolved
playwright/src/main/java/com/microsoft/playwright/impl/BrowserTypeImpl.java
Outdated
Show resolved
Hide resolved
playwright/src/main/java/com/microsoft/playwright/impl/LocatorAssertionsImpl.java
Outdated
Show resolved
Hide resolved
options.recordHarContent = null; | ||
options.recordHarUrlFilter = null; | ||
} else { | ||
if (options.recordHarOmitContent != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where did these checks go?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dropped them accidentally! good catch. I added them back in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, I guess there are no tests for this 😄
playwright/src/main/java/com/microsoft/playwright/impl/BrowserContextImpl.java
Outdated
Show resolved
Hide resolved
playwright/src/main/java/com/microsoft/playwright/impl/BrowserContextImpl.java
Outdated
Show resolved
Hide resolved
playwright/src/main/java/com/microsoft/playwright/impl/BrowserContextImpl.java
Outdated
Show resolved
Hide resolved
@@ -44,7 +43,7 @@ private LocatorAssertionsImpl(Locator locator, boolean isNot) { | |||
public void containsClass(String classname, ContainsClassOptions options) { | |||
ExpectedTextValue expected = new ExpectedTextValue(); | |||
expected.string = classname; | |||
expectImpl("to.contain.class", expected, classname, "Locator expected to contain class", convertType(options, FrameExpectOptions.class)); | |||
expectImpl("to.contain.class", expected, classname, "Locator expected to contain class", convertType(options, FrameExpectOptions.class), "Assert \"containsClass\""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe "assertThat().containsClass()" ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dotnet uses Expect "ToContainClassAsync"
- let's mirror that wording for now. https://github.com/microsoft/playwright-dotnet/blob/1320cc6d92eac27c68040ddf91206be12297989a/src/Playwright/Core/LocatorAssertions.cs#L151
@@ -60,13 +60,6 @@ void shouldBeAbleToGenerateOutline(@TempDir Path tempDir) throws IOException { | |||
assertTrue(outlineSize > noOutlineSize, "Unexpected sizes: " + outlineSize + " noOutline: " + noOutlineSize); | |||
} | |||
|
|||
@Test | |||
@DisabledIf(value="com.microsoft.playwright.TestBase#isChromium", disabledReason="skip") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we now support pdf in ff and wk?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why it can't be tested on the client end even if the place where the error is thrown was moved to the server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addressed
this.baseUrl = new URL(spec); | ||
} catch (MalformedURLException e) { | ||
this.baseUrl = null; | ||
void setRecordHar(Path path, HarContentPolicy policy) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one should not be used anymore.
options.recordHarContent = null; | ||
options.recordHarUrlFilter = null; | ||
} else { | ||
if (options.recordHarOmitContent != null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, I guess there are no tests for this 😄
No description provided.