Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions playwright/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-engine</artifactId>
</dependency>
<dependency>
<groupId>org.junit.platform</groupId>
<artifactId>junit-platform-launcher</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-params</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ Playwright createPlaywright(Playwright.CreateOptions options) {
return playwright;
}

boolean owns(Playwright playwright) {
return playwrightList.contains(playwright);
}

// This is a workaround for JUnit's lack of an "AfterTestRun" hook
// This will be called once after all tests have completed.
Expand Down Expand Up @@ -85,13 +88,14 @@ public Object resolveParameter(ParameterContext parameterContext, ExtensionConte
* @return The Playwright that belongs to the current test.
*/
public static Playwright getOrCreatePlaywright(ExtensionContext extensionContext) {
PlaywrightRegistry registry = PlaywrightRegistry.getOrCreateFor(extensionContext);
Playwright playwright = threadLocalPlaywright.get();
if (playwright != null) {
// A previous launcher run on this thread (e.g. a surefire rerun) has already closed its Playwright.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this comment describes the fall-through case but sits directly above the reuse branch, so it reads as if it justifies the if body — easy for a later reader to "fix" by inverting the condition. Moving it below the if block, or rewording to something like "reuse only while the current run's registry still owns it", would avoid that.

if (playwright != null && registry.owns(playwright)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PlaywrightRegistry implements only AutoCloseable (it stopped implementing ExtensionContext.Store.CloseableResource in #1860), so on JUnit < 5.13 the store never closes it.

Combined with this new owns() gate, that turns a one-off leak into a per-run leak: on JUnit 5.9–5.12, a Surefire run with rerunFailingTestsCount used to reuse the single leaked Playwright across all reruns; now every rerun creates a new Playwright while the previous registry is still never closed, so up to retries × threads driver/node processes stay alive for the lifetime of the JVM.

Suggest having PlaywrightRegistry implement ExtensionContext.Store.CloseableResource as well, so older runtimes actually close the registry and this gate degrades gracefully (5.13+ closes it once via that path and won't double-close).

return playwright;
}

Options options = OptionsExtension.getOptions(extensionContext);
PlaywrightRegistry registry = PlaywrightRegistry.getOrCreateFor(extensionContext);
playwright = registry.createPlaywright(options.playwrightCreateOptions);
threadLocalPlaywright.set(playwright);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* Copyright (c) Microsoft Corporation.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/


package com.microsoft.playwright.junit;

import com.microsoft.playwright.Page;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;

// Not picked up by surefire; TestFixturesRerun runs it through the launcher.
@UsePlaywright
public class RerunFixture {
@Test
void usesPage(Page page) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This launches two real browsers serially (once per launcher run) while blocking a JUnit ForkJoinPool worker on summary.get(), inside a suite configured with junit.jupiter.execution.parallel.mode.classes.default = concurrent. On the 3-vCPU macos-latest runner — where the matrix already excludes WebKit for headroom — that's a meaningful addition.

The regression being guarded (Playwright connection closed) reproduces without a browser, so injecting APIRequestContext here instead of Page (or just making a driver round-trip on Playwright) would cover the same thing much more cheaply.

page.setContent("<title>rerun</title>");
assertEquals("rerun", page.title());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
* Copyright (c) Microsoft Corporation.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/


package com.microsoft.playwright.junit;

import org.junit.jupiter.api.Test;
import org.junit.platform.launcher.LauncherDiscoveryRequest;
import org.junit.platform.launcher.core.LauncherFactory;
import org.junit.platform.launcher.listeners.SummaryGeneratingListener;
import org.junit.platform.launcher.listeners.TestExecutionSummary;

import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.request;

public class TestFixturesRerun {
// Surefire's rerunFailingTestsCount runs the launcher again on the same thread after
// the first run has already closed every Playwright it created.
@Test
void shouldCreateNewPlaywrightForEachLauncherRun() throws Exception {
ExecutorService thread = Executors.newSingleThreadExecutor();
try {
for (int run = 1; run <= 2; run++) {
Future<TestExecutionSummary> summary = thread.submit(() -> runOnLauncher(RerunFixture.class));
assertEquals(1, summary.get().getTestsSucceededCount(), "run " + run + ": " + describeFailures(summary.get()));
}
} finally {
thread.shutdownNow();
}
}

private static String describeFailures(TestExecutionSummary summary) {
StringWriter out = new StringWriter();
summary.printFailuresTo(new PrintWriter(out));
return out.toString();
}

private static TestExecutionSummary runOnLauncher(Class<?> testClass) {
LauncherDiscoveryRequest request = request().selectors(selectClass(testClass)).build();
SummaryGeneratingListener listener = new SummaryGeneratingListener();
LauncherFactory.create().execute(request, listener);
return listener.getSummary();
}
}
7 changes: 7 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@

<dependencyManagement>
<dependencies>
<dependency>
<groupId>org.junit</groupId>
<artifactId>junit-bom</artifactId>
<version>${junit.version}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
<dependency>
<groupId>com.microsoft.playwright</groupId>
<artifactId>driver</artifactId>
Expand Down
7 changes: 7 additions & 0 deletions tools/test-local-installation/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
<compiler.version>1.8</compiler.version>
<gson.version>2.11.0</gson.version>
<junit.version>5.11.0</junit.version>
<junit.platform.version>1.11.0</junit.platform.version>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<websocket.version>1.5.7</websocket.version>
</properties>
Expand Down Expand Up @@ -44,6 +45,12 @@
<version>${junit.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.platform</groupId>
<artifactId>junit-platform-launcher</artifactId>
<version>${junit.platform.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.java-websocket</groupId>
<artifactId>Java-WebSocket</artifactId>
Expand Down
Loading