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.

Sign up for GitHub

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

Jump to bottom

CAMEL-19538: Replaced Thread.sleep() in camel-mllp #14934

Merged
merged 7 commits into from
Sep 11, 2024
Merged

CAMEL-19538: Replaced Thread.sleep() in camel-mllp #14934

merged 7 commits into from
Sep 11, 2024

Conversation

LostArtist
Copy link
Contributor

No description provided.

@github-actions GitHub Actions
Copy link
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟

🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run

  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot.

  • You can label PRs using build-all, build-dependents, skip-tests and test-dependents to fine-tune the checks executed by this PR.

  • Build and test logs are available in the Summary page. Only Apache Camel committers have access to the summary.

  • ⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

orpiske
orpiske requested changes Jul 26, 2024
View reviewed changes
Copy link
Contributor

@orpiske orpiske left a comment

Choose a reason for hiding this comment

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

There are a few problems:

  1. Endlessly waiting in a few places
  2. In general we should avoid having just pollDelay: it's just an uglier, harder to read version of the Thread.sleep. Please look at using atMost or atLeast instead.

...mllp/src/test/java/org/apache/camel/component/mllp/MllpTcpServerConsumerBindTimeoutTest.java Outdated Show resolved Hide resolved
...-mllp/src/test/java/org/apache/camel/component/mllp/MllpTcpServerConsumerConnectionTest.java Outdated Show resolved Hide resolved
components/camel-mllp/src/test/java/org/apache/camel/test/tcp/JavaSocketManualTests.java Outdated Show resolved Hide resolved
orpiske
orpiske requested changes Jul 31, 2024
View reviewed changes
Copy link
Contributor

@orpiske orpiske left a comment

Choose a reason for hiding this comment

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

There's a few more things to adjust

...est/java/org/apache/camel/component/mllp/MllpTcpClientProducerIdleConnectionTimeoutTest.java Show resolved Hide resolved
...mllp/src/test/java/org/apache/camel/component/mllp/MllpTcpServerConsumerBindTimeoutTest.java Outdated Show resolved Hide resolved
...-mllp/src/test/java/org/apache/camel/component/mllp/MllpTcpServerConsumerConnectionTest.java Show resolved Hide resolved
@orpiske
Copy link
Contributor

orpiske commented Aug 1, 2024 •
edited by github-actions bot
Loading

/component-test camel-mllp

Result ❌ The tests failed please check the logs

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 1, 2024

🤖 The Apache Camel test robot will run the tests for you 👍

@oscerd
Copy link
Contributor

oscerd commented Aug 27, 2024

/component-tests mllp

@orpiske orpiske merged commit d57f849 into apache:main Sep 11, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oscerd oscerd oscerd approved these changes

@orpiske orpiske orpiske approved these changes

Assignees
No one assigned
Labels
camel-4 catalog components
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@LostArtist @orpiske @oscerd