Conversation
- Enable parallel test execution in maven-surefire-plugin (4 threads per core) - Refactor PlaywrightClientEdgeCaseTest to use shared client and server - Add @BeforeAll/@afterall lifecycle methods for resource sharing - Fix shared worker cleanup in PlaywrightClient to reset SHARED_WORKER Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR optimizes test performance by enabling parallel test execution and refactoring edge case tests to use shared resources instead of creating new instances for each test method.
Changes:
- Configured Maven Surefire plugin to run test classes in parallel with 4 threads per core
- Fixed a resource cleanup bug where
SHARED_WORKERwasn't reset when closing shared worker instances - Refactored
PlaywrightClientEdgeCaseTestto use class-level shared client and server instances across all test methods
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pom.xml | Added parallel execution configuration to maven-surefire-plugin |
| PlaywrightClient.java | Fixed bug by resetting SHARED_WORKER to null when closing a shared worker instance |
| PlaywrightClientEdgeCaseTest.java | Refactored to use @BeforeAll/@afterall with shared client and server instances instead of per-test initialization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private static PlaywrightClient sharedClient; | ||
| private static CrawlerWebServer sharedServer; | ||
| private static File docRootDir; |
There was a problem hiding this comment.
Shared static resources without synchronization may cause issues when tests run in parallel. Since maven-surefire-plugin is configured with parallel=classes, multiple test classes could run concurrently, and if this test class is instantiated multiple times or accessed concurrently, the shared resources could be accessed by multiple threads without proper synchronization.
| @@ -285,35 +242,11 @@ protected Optional<MimeTypeHelper> getMimeTypeHelper() { | |||
| */ | |||
| @Test | |||
| public void test_execute_concurrentRequests() { | |||
There was a problem hiding this comment.
The test name 'test_execute_concurrentRequests' is misleading because the implementation executes requests sequentially (as noted in the comment on line 245), not concurrently. Consider renaming to 'test_execute_sequentialRequests' or 'test_execute_multipleRequests'.
| public void test_execute_concurrentRequests() { | |
| public void test_execute_sequentialRequests() { |
Summary
Changes Made
parallel=classes,threadCount=4, andperCoreThreadCount=trueSHARED_WORKERto null when closing a shared worker instance, preventing stale references@BeforeAllto initialize shared client and server once per test class@AfterAllto cleanup resources after all tests completeTesting
Breaking Changes
None
🤖 Generated with Claude Code