Skip to content

Migrate few modules to Junit Jupiter #10020

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

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

Conversation

mjd507
Copy link
Contributor

@mjd507 mjd507 commented May 17, 2025

including: spring-integration-amqp, spring-integration-file, spring-integration-groovy

for RabbitAvailable test cases, I am not sure they are working or not, in local all skipped due to no broker available.

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Error: eckstyle] [ERROR] /home/runner/work/spring-integration/spring-integration/spring-integration-amqp/src/test/java/org/springframework/integration/amqp/inbound/ManualAckTests.java:50:74: Using a static member import should be avoided - org.springframework.integration.amqp.inbound.ManualAckTests.ManualAckConfig. [AvoidStaticImport]

Use check Gradle command to verify Checkstyle as well.
Thanks

mjd507 added 2 commits May 18, 2025 11:44
including: `spring-integration-amqp`, `spring-integration-file`, 'spring-integration-groovy'

Signed-off-by: Jiandong Ma <[email protected]>
including: `spring-integration-http`, `spring-integration-jdbc`, `spring-integration-jms`

Signed-off-by: Jiandong Ma <[email protected]>
@mjd507
Copy link
Contributor Author

mjd507 commented May 19, 2025

Hi @artembilan I found one test org.springframework.integration.file.tail.FileTailingMessageProducerTests#testOS annotated with @TailAvailable always failed in my machine (MacOs - support tail command), but always success in our CI pipeline test, not sure our CI container support tail or not.

local failed at this assert assertThat(tailEventLatch.await(20, TimeUnit.SECONDS)).isTrue(); which expect the latch to be released, but actually not. as per the test case, latch will be released if we publish an event.

By checking the OSDelegatingFileTailingMessageProducer, looks we didn't publish any event if no error happened from the std error.

By checking the doc file-tailing, we support FileTailingIdleEvent , and if I try set adapter.setIdleEventInterval(2000); in test, and it will pass.

can confirm can I do it this way, or any better idea.

Thank you!

@artembilan
Copy link
Member

Hey, @mjd507 !

Thank you for digging more about those tests!

The CI is Linux, so ha to pass I guess, e..g. latest SNAPSHOT build: https://ge.spring.io/s/xefov3l7yanqy/tests/task/:spring-integration-file:test/details/org.springframework.integration.file.tail.FileTailingMessageProducerTests/testOS?top-execution=1

I use Windows for development. Still works with my WSL2.
Yes, we observed some problems on MacOS without any clues why.

Please, raise a new PR with whatever you think could be as a fix for that problem.

@artembilan
Copy link
Member

The @RabbitAvailable works as expected.
It is a part of Spring AMQP project and really as designed for JUnit Jupiter.
There are already some tests in this project which use @RabbitAvailable successfully.

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

This is great.
Just some minor comments over there in the code.

Do you know already, @mjd507 , how many modules left with some JUnit 4 tests?

Thank you for great contribution!

@mjd507
Copy link
Contributor Author

mjd507 commented May 19, 2025

Thank you @artembilan , I always expect the comments you leave on my changes, Even though I was doing the relatively easy part, I always learned something useful.

how many modules left with some JUnit 4 tests?

by search org.junit.Test, I see 55 match files, and if I exclude spring-integration-hazelcast, which contains 15 matched files, so only 40 files, most are in spring-integration-jmx module, we don't need migrate spring-integration-hazelcast right?

Signed-off-by: Jiandong Ma <[email protected]>
@artembilan
Copy link
Member

No, we still need to migrate Hazelcast. We have deprecated only some of the components which require CP subsystem. The rest remains as is. And if you say there are some JUnit 4 tests , then we need to migrate them.
No rush, though, because they are just tests.
But tomorrow I have 6.5 GA release . And after that I’m switching to 7.0 where real “hardcore” begins, including the mentioned JUnit 4 utils deprecation.

So, thank you for reporting that, I’ll try to migrate renaming tests myself before releasing tomorrow .

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.

2 participants