Skip to content

Migrate PubSub removing flaky test#1407

Merged
dapr-bot merged 10 commits intodapr:masterfrom
mcruzdev:issue-1392
Jul 14, 2025
Merged

Migrate PubSub removing flaky test#1407
dapr-bot merged 10 commits intodapr:masterfrom
mcruzdev:issue-1392

Conversation

@mcruzdev
Copy link
Contributor

@mcruzdev mcruzdev commented Jun 7, 2025

Description

Use testcontainers for PubSub tests.

The main goal is to remove the following flaky test https://github.com/dapr/java-sdk/actions/runs/13773009141/job/38515768267#step:19:13432.

Closes one of the flaky test links related on the #1258 issue.

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #[issue number]

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

@mcruzdev
Copy link
Contributor Author

mcruzdev commented Jun 7, 2025

@artur-ciocanu @cicoyle @salaboy

Trying to move the following test to use Testcontainers I got a problem.

public void testPubSubBulkSubscribe() throws Exception {
DaprRun daprRun = closeLater(startDaprApp(

When running the test from PubSub the following assert works well:

I have 1 message with the first message containing 10 statuses.

But using in-memory (Testcontainers) I have 10 messages and each one with 1 statuses.

I would like to know if the in-memory PubSub supports bulk message? The PubSub from DaprRun uses Redis

@mcruzdev mcruzdev marked this pull request as ready for review June 7, 2025 14:06
@mcruzdev mcruzdev requested review from a team as code owners June 7, 2025 14:06
@artur-ciocanu
Copy link
Contributor

@mcruzdev I will try to review ASAP, but could you please check why the ITs are failing?

@mcruzdev
Copy link
Contributor Author

@mcruzdev I will try to review ASAP, but could you please check why the ITs are failing?

It is another mapped flaky test

Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
@mcruzdev
Copy link
Contributor Author

Error, I need to change the assert...

expected: 
  [io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@5bbe4e8f,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@dc7818,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@b2fdfa8,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@4613e544,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@abac76b4,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@e8f49fdd,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@bf3b3eb2,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@9843cc0f,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@3346a35b]
 but was: 
  [io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@f7ce0e8f,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@5bbe4e8f,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@dc7818,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@b2fdfa8,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@4613e544,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@abac76b4,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@e8f49fdd,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@bf3b3eb2,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@9843cc0f,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@3346a35b]

@artur-ciocanu
Copy link
Contributor

@mcruzdev could you please check what is wrong with the tests, I see this error:

Error:    DaprPubSubIT.testLongValues:578 » Runtime org.opentest4j.AssertionFailedError: 
expected: 
  [io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@5bbe4e8f,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@dc7818,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@b2fdfa8,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@4613e544,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@abac76b4,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@e8f49fdd,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@bf3b3eb2,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@9843cc0f,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@3346a35b]
 but was: 
  [io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@f7ce0e8f,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@5bbe4e8f,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@dc7818,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@b2fdfa8,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@4613e544,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@abac76b4,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@e8f49fdd,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@bf3b3eb2,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@9843cc0f,
      io.dapr.it.pubsub.http.PubSubIT$ConvertToLong@3346a35b]

artur-ciocanu and others added 4 commits June 26, 2025 21:56
Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
Copy link
Contributor

@artur-ciocanu artur-ciocanu left a comment

Choose a reason for hiding this comment

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

@mcruzdev the PR looks good. There are a few things that I think should be addressed:

  • Please make sure that you use logger instead of System.out, I have tried to add comments to all the places, but please double check and make sure that we use logger everywhere
  • The Dapr Jackson serialzer could replaced with CustomizableObjectSerializer instead of creating an inline class
  • I see that there are quite a few Thread.sleep() invocations, I am wondering if there is a way to avoid them. If not I am OK with them.

Please take a look and let me know your thoughts.

Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
@mcruzdev mcruzdev requested a review from artur-ciocanu July 7, 2025 18:43
@mcruzdev
Copy link
Contributor Author

Hi @artur-ciocanu we really need a custom ObjectSerializer, using a CustomizableObjectSerializer that allow the ObjectMapper be customizable are broking some tests.

I just copied the old behavior to the new way to write tests (using testcontainers to instantiate dapr).

Copy link
Contributor

@artur-ciocanu artur-ciocanu left a comment

Choose a reason for hiding this comment

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

@mcruzdev you made my day! Really nice patch, thank you! 🙇

@dapr-bot dapr-bot merged commit 7a4f7d3 into dapr:master Jul 14, 2025
11 of 12 checks passed
@codecov
Copy link

codecov bot commented Jul 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.34%. Comparing base (d759c53) to head (3806ae6).
Report is 179 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1407      +/-   ##
============================================
+ Coverage     76.91%   78.34%   +1.42%     
- Complexity     1592     1878     +286     
============================================
  Files           145      234      +89     
  Lines          4843     5832     +989     
  Branches        562      601      +39     
============================================
+ Hits           3725     4569     +844     
- Misses          821      943     +122     
- Partials        297      320      +23     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mcruzdev mcruzdev deleted the issue-1392 branch July 14, 2025 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants