Skip to content

Fix Playwright tests failing when run together#1906

Merged
jnioche merged 4 commits into
mainfrom
playwrightTests
May 15, 2026
Merged

Fix Playwright tests failing when run together#1906
jnioche merged 4 commits into
mainfrom
playwrightTests

Conversation

@jnioche
Copy link
Copy Markdown
Contributor

@jnioche jnioche commented May 14, 2026

The issue was that ProtocolTest and PageActionsLiveTest both extended AbstractProtocolTest and shared the same static Jetty server instance. When running tests together, the server would be stopped by one test class before the other could use it, causing ERR_CONNECTION_REFUSED errors.

This fix creates a new BasePlaywrightTest class specifically for the Playwright module that gives each test class its own isolated server instance.

Changes:

  • Created BasePlaywrightTest with proper server management
  • Updated ProtocolTest to extend BasePlaywrightTest
  • Updated PageActionsLiveTest to extend BasePlaywrightTest
  • Removed redundant getHandlers() overrides

Testing
• Ran full test suite: mvn test -pl external/playwright - PASS
• Ran individual tests: mvn test -pl external/playwright -Dtest=PageActionsLiveTest - PASS
• Ran individual tests: mvn test -pl external/playwright -Dtest=ProtocolTest - PASS

The issue was that ProtocolTest and PageActionsLiveTest both extended
AbstractProtocolTest and shared the same static Jetty server instance.
When running tests together, the server would be stopped by one test
class before the other could use it, causing ERR_CONNECTION_REFUSED errors.

This fix creates a new BasePlaywrightTest class specifically for the
Playwright module that gives each test class its own isolated server instance.

Changes:
- Created BasePlaywrightTest with proper server management
- Updated ProtocolTest to extend BasePlaywrightTest
- Updated PageActionsLiveTest to extend BasePlaywrightTest
- Removed redundant getHandlers() overrides

Fixes: #issue-number (if applicable)

Generated by Mistral Vibe.
Co-Authored-By: Mistral Vibe <vibe@mistral.ai>
@jnioche jnioche added the bug label May 14, 2026
@jnioche jnioche marked this pull request as draft May 14, 2026 15:43
Signed-off-by: Julien Nioche <julien@digitalpebble.com>
@jnioche
Copy link
Copy Markdown
Contributor Author

jnioche commented May 14, 2026

something needs fixing - ignore this PR for now

Error:  Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.15.0:testCompile (default-testCompile) on project stormcrawler-core: Compilation failure
Error:  /home/runner/work/stormcrawler/stormcrawler/core/src/test/java/org/apache/stormcrawler/protocol/AbstractProtocolTest.java:[48,30] non-static method getHandlers() cannot be referenced from a static context

@sigee
Copy link
Copy Markdown
Member

sigee commented May 15, 2026

If you don't mind @jnioche , I finished your PR by reverting 90% of it. :)
The main issue was in the AbstractProtocolTest the AfterAll stopJetty() method stopped the jetty server, but the variable remained initialized after the first subclass test finished (E.g.: HttpProtocolProxyConcurrencyTest).
When the second subclass wanted to initialize the jetty as it was in the BeforeEach initJetty(), it checked the httpServer is already initialized (but it wasn't running) so skipped the reinitialization.

@rzo1 rzo1 marked this pull request as ready for review May 15, 2026 12:33
@jnioche
Copy link
Copy Markdown
Contributor Author

jnioche commented May 15, 2026

If you don't mind @jnioche , I finished your PR by reverting 90% of it. :) The main issue was in the AbstractProtocolTest the AfterAll stopJetty() method stopped the jetty server, but the variable remained initialized after the first subclass test finished (E.g.: HttpProtocolProxyConcurrencyTest). When the second subclass wanted to initialize the jetty as it was in the BeforeEach initJetty(), it checked the httpServer is already initialized (but it wasn't running) so skipped the reinitialization.

thanks @sigee

apologies for the half baked PR, I was evaluating Mistral's vibe tool and thought I would try it on a simple PR. Meant to have a closer look at it but you beat me to it.

@jnioche jnioche merged commit 59cb3fd into main May 15, 2026
2 checks passed
@jnioche jnioche deleted the playwrightTests branch May 15, 2026 12:57
@jnioche jnioche added this to the 3.6.0 milestone May 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants