-
Notifications
You must be signed in to change notification settings - Fork 276
fix: Replaced classLoader in DriverJar #1811
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
Changes from 3 commits
a7830eb
2b8f04d
3c925b1
04b40cf
999cef6
2b34498
ef25b16
3da9be6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| #!/bin/bash | ||
|
|
||
| set -e | ||
| set +x | ||
|
|
||
| cd "$(dirname "$0")" | ||
| mvn package -D skipTests --no-transfer-progress | ||
| java -jar target/test-spring-classloader*.jar |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
| <modelVersion>4.0.0</modelVersion> | ||
| <parent> | ||
| <groupId>org.springframework.boot</groupId> | ||
| <artifactId>spring-boot-starter-parent</artifactId> | ||
| <version>2.4.3</version> | ||
| </parent> | ||
| <groupId>com.microsoft.playwright</groupId> | ||
| <artifactId>test-spring-classloader</artifactId> | ||
| <version>1.50.0-SNAPSHOT</version> | ||
| <name>Test Playwright With Spring Boot</name> | ||
| <properties> | ||
| <spring.version>2.4.3</spring.version> | ||
| </properties> | ||
| <dependencies> | ||
| <dependency> | ||
| <groupId>org.springframework.boot</groupId> | ||
| <artifactId>spring-boot-starter</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>com.microsoft.playwright</groupId> | ||
| <artifactId>playwright</artifactId> | ||
| <version>${project.version}</version> | ||
| </dependency> | ||
| </dependencies> | ||
| <build> | ||
| <plugins> | ||
| <plugin> | ||
| <groupId>org.springframework.boot</groupId> | ||
| <artifactId>spring-boot-maven-plugin</artifactId> | ||
| </plugin> | ||
| </plugins> | ||
| </build> | ||
| </project> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| package com.microsoft.playwright.springboottest; | ||
|
|
||
| import com.microsoft.playwright.*; | ||
| import org.springframework.boot.CommandLineRunner; | ||
| import org.springframework.boot.SpringApplication; | ||
| import org.springframework.boot.autoconfigure.SpringBootApplication; | ||
|
|
||
| import java.util.concurrent.CompletableFuture; | ||
|
|
||
| @SpringBootApplication | ||
| public class TestApp implements CommandLineRunner { | ||
|
|
||
| public static void main(String[] args) { | ||
| SpringApplication.run(TestApp.class, args); | ||
| } | ||
|
|
||
| public void run(String... args) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we just run existing
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The key difference is that the new test uses CompletableFuture for asynchronous execution. While the existing test works fine even in the Docker environment, i encountered an issue where DriverJar could not be read when executed from a new thread created by CompletableFuture inside the Docker container. That’s why this additional test was introduced.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that case you let's just modify existing test to either always run playwright asynchronously or, if you want to keep testing sync code path as well, do it sync/async based on a command line flag and pass the flag only when running in docker. Something like this: public static void main(String[] args) {
if ("--async".equals(args[0])) {
CompletableFuture<Void> voidCompletableFuture = CompletableFuture.runAsync(() -> {
SpringApplication.run(TestApp.class, args);
});
voidCompletableFuture.join();
} else {
SpringApplication.run(TestApp.class, args);
}
}Would that work?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When running with --async, Spring Boot uses the main thread’s context class loader to load auto-configuration classes. However, in the asynchronous thread, this context class loader is not correctly inherited, which causes it to fail to locate the configuration files. 11:48:21.637 [ForkJoinPool.commonPool-worker-1] ERROR org.springframework.boot.SpringApplication - Application run failed
java.lang.IllegalArgumentException: No auto configuration classes found in META-INF/spring.factories. If you are using a custom packaging, make sure that file is correct.
at org.springframework.util.Assert.notEmpty(Assert.java:470)
at org.springframework.boot.autoconfigure.AutoConfigurationImportSelector.getCandidateConfigurations(AutoConfigurationImportSelector.java:180)
at org.springframework.boot.autoconfigure.AutoConfigurationImportSelector.getAutoConfigurationEntry(AutoConfigurationImportSelector.java:123)
at org.springframework.boot.autoconfigure.AutoConfigurationImportSelector$AutoConfigurationGroup.process(AutoConfigurationImportSelector.java:434)
at org.springframework.context.annotation.ConfigurationClassParser$DeferredImportSelectorGrouping.getImports(ConfigurationClassParser.java:879)
at org.springframework.context.annotation.ConfigurationClassParser$DeferredImportSelectorGroupingHandler.processGroupImports(ConfigurationClassParser.java:809)
at org.springframework.context.annotation.ConfigurationClassParser$DeferredImportSelectorHandler.process(ConfigurationClassParser.java:780)
at org.springframework.context.annotation.ConfigurationClassParser.parse(ConfigurationClassParser.java:193)
at org.springframework.context.annotation.ConfigurationClassPostProcessor.processConfigBeanDefinitions(ConfigurationClassPostProcessor.java:330)
at org.springframework.context.annotation.ConfigurationClassPostProcessor.postProcessBeanDefinitionRegistry(ConfigurationClassPostProcessor.java:246)
at org.springframework.context.support.PostProcessorRegistrationDelegate.invokeBeanDefinitionRegistryPostProcessors(PostProcessorRegistrationDelegate.java:311)
at org.springframework.context.support.PostProcessorRegistrationDelegate.invokeBeanFactoryPostProcessors(PostProcessorRegistrationDelegate.java:112)
at org.springframework.context.support.AbstractApplicationContext.invokeBeanFactoryPostProcessors(AbstractApplicationContext.java:745)
at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:563)
at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:767)
at org.springframework.boot.SpringApplication.refresh(SpringApplication.java:759)
at org.springframework.boot.SpringApplication.refreshContext(SpringApplication.java:426)
at org.springframework.boot.SpringApplication.run(SpringApplication.java:326)
at org.springframework.boot.SpringApplication.run(SpringApplication.java:1311)
at org.springframework.boot.SpringApplication.run(SpringApplication.java:1300)
at com.microsoft.playwright.springboottest.TestApp.lambda$main$0(TestApp.java:16)
at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1804)
at java.base/java.util.concurrent.CompletableFuture$AsyncRun.exec(CompletableFuture.java:1796)
at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:387)
at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1312)
at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1843)
at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1808)
at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:188)While it’s possible to manually pass the main thread’s context class loader to the async thread like below, doing so defeats the original purpose of testing the class loader behavior. public static void main(String[] args) {
if (args.length == 0) {
SpringApplication.run(TestApp.class, args);
}
else {
if ("--async".equals(args[0])) {
ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader();
CompletableFuture<Void> voidCompletableFuture = CompletableFuture.runAsync(() -> {
Thread.currentThread().setContextClassLoader(contextClassLoader);
SpringApplication.run(TestApp.class, args);
});
voidCompletableFuture.join();
}
}
}For this reason, I think separating the test into a different package would make the intention clearer.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I meant to just share the implementation of the actual test method, I guess it should have been something like this: @Override
public void run(String... args) {
if ("--async".equals(args[0])) {
runAsync();
} else {
runSync();
}
}
private void runAsync() {
CompletableFuture<Void> voidCompletableFuture = CompletableFuture.runAsync(() -> {
runSync();
});
voidCompletableFuture.join();
}
private void runSync() {
try (Playwright playwright = Playwright.create()) {
BrowserType browserType = getBrowserTypeFromEnv(playwright);
System.out.println("Running test with " + browserType.name());
Browser browser = browserType.launch();
BrowserContext context = browser.newContext();
Page page = context.newPage();
System.out.println(page.evaluate("'SUCCESS: did evaluate in page'"));
}
}Basically the logic from the existing test is in |
||
| // use CompletableFuture to run Playwright asynchronously | ||
| CompletableFuture<Void> voidCompletableFuture = CompletableFuture.runAsync(() -> { | ||
| try (Playwright playwright = Playwright.create()) { | ||
| System.out.println("Playwright classLoader test started, waiting for completion..."); | ||
| BrowserType browserType = getBrowserTypeFromEnv(playwright); | ||
| System.out.println("Running test with " + browserType.name()); | ||
| Browser browser = browserType.launch(); | ||
| BrowserContext context = browser.newContext(); | ||
| Page page = context.newPage(); | ||
| System.out.println(page.evaluate("'SUCCESS: did evaluate in page'")); | ||
| } catch (Exception e) { | ||
| System.out.println("FAILED: " + e.toString()); | ||
| for (StackTraceElement ste : e.getStackTrace()) { | ||
| System.out.println("\tat " + ste); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| System.out.println("Playwright classLoader test is running asynchronously, main thread will wait for it to complete."); | ||
|
|
||
| voidCompletableFuture.join(); | ||
|
|
||
| System.out.println("Playwright classLoader test completed."); | ||
|
|
||
| } | ||
|
|
||
| static BrowserType getBrowserTypeFromEnv(Playwright playwright) { | ||
| String browserName = System.getenv("BROWSER"); | ||
|
|
||
| if (browserName == null) { | ||
| browserName = "chromium"; | ||
| } | ||
|
|
||
| switch (browserName) { | ||
| case "webkit": | ||
| return playwright.webkit(); | ||
| case "firefox": | ||
| return playwright.firefox(); | ||
| case "chromium": | ||
| return playwright.chromium(); | ||
| default: | ||
| throw new IllegalArgumentException("Unknown browser: " + browserName); | ||
| } | ||
| } | ||
|
|
||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
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.
To avoid the docker test failures let's just add
docker stop "${CONTAINER_ID}"to this (and the new) step to shutdown the container after the test has finished. Or better yet, since the container is the same in both steps, you can move its creation in a separate step and then use its CONTAINER_ID without relaunching, something likeThere 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 added example code and stop step