Skip to content
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

Upgrade gradle version in use to 7.6 #200

Closed
wants to merge 1 commit into from

Conversation

KushalP
Copy link
Contributor

@KushalP KushalP commented Feb 19, 2024

Command that was used to produce the upgrade diff:

./gradlew wrapper --gradle-version=7.6

Command that was used to produce the upgrade diff:

```
./gradlew wrapper --gradle-version=7.6
```
@holzensp
Copy link
Collaborator

Digging into that flakey test.

@odenix
Copy link
Contributor

odenix commented Feb 19, 2024

@holzensp All tests that use PackageServer are flaky.
Higher Gradle versions just seem better at exposing the flakiness.
The proper fix is to bind PackageServer to a dynamic port.
(Currently the port is hardcoded to 12110.)

I'd be happy to take this on. It's a natural follow-up to #157.

@holzensp
Copy link
Collaborator

@holzensp All tests that use PackageServer are flaky. Higher Gradle versions just seem better at exposing the flakiness. The proper fix is to bind PackageServer to a dynamic port. (Currently the port is hardcoded to 12110.)

Yes, it appears to be increased concurrency with which the tests are run. We are no playing around with assigning different ports.

I'd be happy to take this on. It's a natural follow-up to #157.

Thanks for the offer, but work is underway as we speak, so let's avoid the duplication of effort. Press on as if these are fixed!

@odenix
Copy link
Contributor

odenix commented Feb 20, 2024

@holzensp FYI every clean build of the main branch fails with connection refused errors on my machine. I always need to run the build a second time to get past the flaky tests. In my experience, binding test servers to dynamic ports (or cutting out the network layer) is the only good solution. It's pretty easy once all requests are routed through the same HttpClient code.

I'm ready to send a PR for #157 but have been holding back to give you guys a chance to catch up with other PRs.

@holzensp
Copy link
Collaborator

Much appreciated @translatenix. @KushalP is also working on a PR to avoid port binding conflicts when tests are run in parallel. The tests really aren't rigid enough. Your #157 will likely help with remaining fickleness of these tests. Reviewing today (promise).

@KushalP
Copy link
Contributor Author

KushalP commented Feb 23, 2024

Closing this as it had been superseded by #58.

@KushalP KushalP closed this Feb 23, 2024
@KushalP KushalP deleted the upgrade-gradle-7.6 branch February 23, 2024 11:30
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.

3 participants