Skip to content

Conversation

mxschmitt
Copy link
Member

  • GHA Timeouts were increased to 60min
  • Verified that retries are working, weird that they didn't seem to work on CI
  • Introduced PW_JAVA_INTERNAL_SILENT_INSTALL to have no confusing output in the test output - Overriding the stderr/out from inside the test does not work, since ProcessBuilder is based on file descriptors.
  • Had to add slf4j-simple in order to get rid of:
SLF4J(W): No SLF4J providers were found.
SLF4J(W): Defaulting to no-operation (NOP) logger implementation
SLF4J(W): See https://www.slf4j.org/codes.html#noProviders for further details.

tbd. Lets see if increasing the timeouts helps to make the tests pass.

jobs:
dev:
timeout-minutes: 30
timeout-minutes: 60
Copy link
Member

Choose a reason for hiding this comment

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

Let's do it only for channels that we know are slow?

<groupId>org.java-websocket</groupId>
<artifactId>Java-WebSocket</artifactId>
</dependency>
<dependency>
Copy link
Member

Choose a reason for hiding this comment

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

let's remove?

<scope>test</scope>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
Copy link
Member

Choose a reason for hiding this comment

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

Let's add comment that it's just for websocket to stop polluting the output with warnings.

<properties>
<configurationParameters>
junit.jupiter.execution.parallel.enabled = true
junit.jupiter.execution.parallel.enabled = false
Copy link
Member

Choose a reason for hiding this comment

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

stray change?

// Make sure the browsers are not installed yet by pointing at an empty dir.
env.put("PLAYWRIGHT_BROWSERS_PATH", tmpDir.toString());
env.put("PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD", "false");
env.put("PW_JAVA_INTERNAL_SILENT_INSTALL", "true");
Copy link
Member

Choose a reason for hiding this comment

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

Let's not do it for now. If we do it, let's make it a public option?

@yury-s yury-s self-requested a review March 17, 2025 17:57
@mxschmitt mxschmitt force-pushed the test/warning-cleanups branch from 1a11c01 to fafef05 Compare March 17, 2025 18:17
@mxschmitt mxschmitt force-pushed the test/warning-cleanups branch from a80bbc3 to 25e478f Compare March 17, 2025 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants