Skip to content

Conversation

@wlfgang
Copy link

@wlfgang wlfgang commented Oct 24, 2025

Description

Added a simple withAppProtocol method to the DaprContainer test container which allows the user to specify --app-protocol either HTTP or GRPC, allowing projects to exercise gRPC services from integration tests.

Issue reference

This will close #1578

Checklist

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

Let me know if there's documentation that should be updated, I can add that.

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

@wlfgang wlfgang requested review from a team as code owners October 24, 2025 18:23
@mcruzdev
Copy link
Contributor

Hi @wlfgang thanks for this pull request, you need to sign-off your commit.

@wlfgang wlfgang force-pushed the 1578-testcontainer-grpc-support branch from 2c7e916 to e782e6b Compare October 24, 2025 21:00
@wlfgang
Copy link
Author

wlfgang commented Oct 24, 2025

Hi @wlfgang thanks for this pull request, you need to sign-off your commit.

Thanks @mcruzdev -- I've added the DCO to the commit.

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.

Nicely done! Thanks a lot for your contribution @wlfgang!

@codecov
Copy link

codecov bot commented Oct 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.53%. Comparing base (d759c53) to head (a36457e).
⚠️ Report is 234 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1586      +/-   ##
============================================
+ Coverage     76.91%   78.53%   +1.62%     
- Complexity     1592     1936     +344     
============================================
  Files           145      216      +71     
  Lines          4843     5871    +1028     
  Branches        562      659      +97     
============================================
+ Hits           3725     4611     +886     
- Misses          821      923     +102     
- Partials        297      337      +40     

☔ 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.

@artur-ciocanu
Copy link
Contributor

@dapr/maintainers-java-sdk and @dapr/approvers-java-sdk let me know if you have any concerns merging this PR.

try (DaprContainer daprContainer = new DaprContainer(DAPR_RUNTIME_IMAGE_TAG)
.withAppName("dapr-app")) {
daprContainer.configure();
assertNull(daprContainer.getAppPort());
Copy link
Collaborator

Choose a reason for hiding this comment

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

@wlfgang I think for backward compatibility and to make it easier for the user configuring the DaprContainer, HTTP should be still the default, hence if nothing is set you still get HTTP.

Copy link
Author

Choose a reason for hiding this comment

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

@salaboy I agree completely, and this is still true; HTTP will be used by default if the user doesn't call the method. For withAppProtocol I mirrored the existing implementation of withAppPort - if you don't call these methods, their fields won't be assigned, and the --app-port and --app-protocol arguments won't be specified to dapr.

If you like, I can change it to initialize appProtocol rather than relying on a null, which may be considered better practice, and the behavior would remain the same.

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.

Add withAppProtocol to testcontainer

4 participants