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

Fix flaky ServerStatusManagerIntegrationTest (2) #949

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ikhoon
Copy link
Contributor

@ikhoon ikhoon commented Apr 23, 2024

Motivation:

This PR tries to solve the flakiness of ServerStatusManagerIntegrationTest in a different way. See #948

This workaround minimizes port collisions by ensuring that all ports are available when a Central Dogma cluster starts.

Modifications:

  • Check if all ports are open when restarting CentralDogmaReplicationExtension.

Result:

Less flakiness

Motivation:

This PR tries to solve the flakiness of `ServerStatusManagerIntegrationTest`
in a different way. See line#948

This workaround minimizes port collisions by ensuring that all ports are
available when a Central Dogma cluster starts.

Modifications:

- Check if all ports are open when restarting
  `CentralDogmaReplicationExtension`.

Result:

Less flakiness
@ikhoon ikhoon added the cleanup label Apr 23, 2024
@ikhoon ikhoon added this to the 0.66.0 milestone Apr 23, 2024
Copy link

codecov bot commented Apr 23, 2024

Codecov Report

Attention: Patch coverage is 88.23529% with 4 lines in your changes missing coverage. Please review.

Project coverage is 70.54%. Comparing base (aae194c) to head (96a680f).
Report is 19 commits behind head on main.

Current head 96a680f differs from pull request most recent head 64990fc

Please upload reports for the commit 64990fc to get more accurate results.

Files Patch % Lines
...ing/internal/CentralDogmaReplicationExtension.java 88.23% 1 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #949      +/-   ##
============================================
+ Coverage     66.86%   70.54%   +3.68%     
- Complexity     3529     3684     +155     
============================================
  Files           370      376       +6     
  Lines         14531    14796     +265     
  Branches       1563     1591      +28     
============================================
+ Hits           9716    10438     +722     
+ Misses         3936     3428     -508     
- Partials        879      930      +51     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@minwoox
Copy link
Member

minwoox commented Apr 26, 2024

Oops, could you check the failure?
https://github.com/line/centraldogma/actions/runs/8798020766/job/24144132408?pr=949#step:6:494

Copy link
Contributor

@jrhee17 jrhee17 left a comment

Choose a reason for hiding this comment

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

I don't have a better idea on how to deal with this flakiness 😅 Looks good once the flakiness is handled 👍 👍 👍

@minwoox minwoox modified the milestones: 0.66.0, 0.67.0 Jun 13, 2024
@ikhoon
Copy link
Contributor Author

ikhoon commented Jun 13, 2024

I tried to increase the max attempts although it won't be a perfect solution.

Copy link
Member

@minwoox minwoox left a comment

Choose a reason for hiding this comment

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

👍 👍 👍

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.

None yet

3 participants