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

ci: fix flaky testcontainers tests #4712

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ndr-brt
Copy link
Member

@ndr-brt ndr-brt commented Jan 10, 2025

What this PR changes/adds

Refactor EndToEnd tests to avoid flaky test executions when testcontainers is used.
Concepts:

  • EmbeddedRuntime configurations are passed lazily through configurationProviders
  • The configuration values in Participant subclasses are generated lazily (using the provided LazySupplier) during the EDC runtime startup so they will take into account eventual testcontainers ports used

Why it does that

avoid test flakyness by design

Further notes

  • there still are two potential pityfalls in Participant: managementEndpoint and protocolEndpoint that are currently instantiated statically while they should follow the same pattern. It will be fixed in a subsequent PR to avoid clogging this one
  • I will also provide some guidance in the documentation that will be useful also for downstream projects

Linked Issue(s)

Part of #4710

Please be sure to take a look at the contributing guidelines and our etiquette for pull requests.

@ndr-brt ndr-brt added refactoring Cleaning up code and dependencies flaky test labels Jan 10, 2025
@ndr-brt ndr-brt marked this pull request as draft January 10, 2025 13:34
@ndr-brt ndr-brt force-pushed the fix-flaky-testcontainers-tests branch from a132971 to 5fd6b4b Compare January 10, 2025 13:38
@ndr-brt ndr-brt marked this pull request as ready for review January 10, 2025 14:08
@rafaelmag110
Copy link
Contributor

rafaelmag110 commented Jan 10, 2025

Could we have the getFreePort lazily supplying ports instead? Does that make sense? Would that be simpler? I'm trying to get my head around this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flaky test refactoring Cleaning up code and dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants