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

[WFCORE-6685] CLIEmbedServerTestCase.testStdOutEcho fails intermittently #5877

Merged
merged 1 commit into from
Feb 26, 2024

Conversation

lvydra
Copy link
Contributor

@lvydra lvydra commented Feb 21, 2024

@github-actions github-actions bot added the deps-ok Dependencies have been checked, and there are no significant changes label Feb 21, 2024
Comment on lines 738 to 739
} else {
Thread.sleep(20);
}
Copy link
Collaborator

@yersan yersan Feb 22, 2024

Choose a reason for hiding this comment

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

Thanks for adding this, although I would suggest a longer pause, let's say 1 second or half one. Using TimeUnit makes it more readable, but that's also completely optional e.g TimeUnit.MILLISECONDS.sleep(500)

The else can be removed since the break will do the work, that's optional too.

None of these notes would prevent merging it as it is, so I leave it up to you to apply the changes. Otherwise, I will approve and merge

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for review @yersan, I have added suggested changes.

@yersan yersan added the ready-for-merge This PR is ready to be merged and fulfills all requirements label Feb 22, 2024
@yersan yersan merged commit 1acb933 into wildfly:main Feb 26, 2024
12 checks passed
@yersan
Copy link
Collaborator

yersan commented Feb 26, 2024

Thanks @lvydra

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deps-ok Dependencies have been checked, and there are no significant changes ready-for-merge This PR is ready to be merged and fulfills all requirements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants