-
Notifications
You must be signed in to change notification settings - Fork 208
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
Add actor testcontainer tests #1192
base: master
Are you sure you want to change the base?
Conversation
@salaboy I have currently the problem that the test fails with the following exception when trying to connect to the Dapr gRPC port.
Not sure why it uses port 50001, because this is the internal port. I think normally it should use the mapped port. Maybe you can have a look, if the test is not correct configured? |
@akkie thanks a lot of this.. give me some time to look into this.. check the DCO, we need that to approve the PR. Click on the Details link to see the steps to fix it. |
@salaboy Is it OK if I do a force push regarding the update of the DCO? |
@akkie yeah.. that is your fork.. so it is ok |
ee83cff
to
bec4bd2
Compare
@akkie would you mind adding me as a collaborator to your fork? I am working on a fix, but I would love to push to your fork.. if not I can send you a patch to apply to your fork with the fix |
Done |
@akkie I will push two commits to your fork, I am stuck with a new error now.. but at least the connection is working now. |
Now I am stuck with this:
But the connection is working as far as I can tell. |
This is strange.. because it looks like we are hitting this: dapr/dapr#6783 @artursouza do you know if these examples are executed as part of the tests? |
@salaboy This is exactly the message we get with our testcontainer setup:
We use .NET and not Java. |
Ok so I think we are up to something here
- Blog: http://salaboy.com <http://salaboy.wordpress.com>
- Github user: http://github.com/salaboy
- Twitter: http://twitter.com/salaboy
- Mauricio "Salaboy" Salatino -
…On Wed, 22 Jan 2025 at 20:11, Christian Kaps ***@***.***> wrote:
@salaboy <https://github.com/salaboy> This is exactly the message we get
with our testcontainer setup:
´´´
fails to send binding event to http app channel, status code: 500 body:
Dapr.DaprApiException: error invoke actor method: did not find address for
actor
´´´
We use .NET and not Java.
—
Reply to this email directly, view it on GitHub
<#1192 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACCMXUKSIIUSSF7NQM2YJT2L73O5AVCNFSM6AAAAABVI4UL56VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMBYGE3TANRUHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@akkie I am curious.. are you testing with an in-memory statestore? |
@salaboy Yes, we are using Redis. |
@akkie if you pull the code that I push can you check that you are getting the same results? |
@salaboy You mean running the tests? If I run them, yes, I get the same result:
|
@akkie good news.. i think that I found the issue..
I will try to fix this and push again. |
@salaboy Any news regarding the issue? |
I am still fighting with this.. for some
Reason after fixing all the connection issues, the actor is not registered
in the placement service. I am trying to figure out why
- Blog: http://salaboy.com <http://salaboy.wordpress.com>
- Github user: http://github.com/salaboy
- Twitter: http://twitter.com/salaboy
- Mauricio "Salaboy" Salatino -
…On Thu, 30 Jan 2025 at 10:12, Christian Kaps ***@***.***> wrote:
@salaboy <https://github.com/salaboy> Any news regarding the issue?
—
Reply to this email directly, view it on GitHub
<#1192 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACCMXUL5XKLZCHGK7LKWE32NH3JFAVCNFSM6AAAAABVI4UL56VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMRUGA2TQNZRGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
The funny thing is that the actor example in the Java SDK is working , but
that example uses two sidecars instead of one.. and I am not entirely sure
if that placement service is configured differently
- Blog: http://salaboy.com <http://salaboy.wordpress.com>
- Github user: http://github.com/salaboy
- Twitter: http://twitter.com/salaboy
- Mauricio "Salaboy" Salatino -
…On Thu, 30 Jan 2025 at 10:59, Mauricio Salatino ***@***.***> wrote:
I am still fighting with this.. for some
Reason after fixing all the connection issues, the actor is not registered
in the placement service. I am trying to figure out why
- Blog: http://salaboy.com <http://salaboy.wordpress.com>
- Github user: http://github.com/salaboy
- Twitter: http://twitter.com/salaboy
- Mauricio "Salaboy" Salatino -
On Thu, 30 Jan 2025 at 10:12, Christian Kaps ***@***.***>
wrote:
> @salaboy <https://github.com/salaboy> Any news regarding the issue?
>
> —
> Reply to this email directly, view it on GitHub
> <#1192 (comment)>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AACCMXUL5XKLZCHGK7LKWE32NH3JFAVCNFSM6AAAAABVI4UL56VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMRUGA2TQNZRGY>
> .
> You are receiving this because you were mentioned.Message ID:
> ***@***.***>
>
|
@salaboy and @akkie I have found what's the culprit. Please check #1202. The TL;DR is that I will try to have a quick fix just to unblock this PR, but in general I think CC: @artursouza @cicoyle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sdk-tests/src/test/java/io/dapr/it/testcontainers/TestDaprActorsConfiguration.java
Show resolved
Hide resolved
@artur-ciocanu Thanks for working on that. I fixed the formatting issues. Is there anything I need to do to get the tests running locally? They still failing for me with the message: api error: code = Internal desc = error invoke actor method: did not find address for actor TestActor/789215ad-9328-4019-8c86-6a4b6cc664a7 |
@akkie I think we are hitting the same issue as we hit with PubSub.. we need to investigate this further.. My expectation is that if we run this application in a Kubernetes Cluster, everything works.. so the issue is related with the sequence in which both the app and the sidecar gets bootstrapped by testcontainers. |
Ok I will check this today
…On Mon, Feb 17, 2025 at 7:59 PM Artur Souza ***@***.***> wrote:
If you run the test a few times in a row, it starts failing right at the
second run:
image.png (view on web)
<https://github.com/user-attachments/assets/1c7c2c4c-c368-471a-8f2b-f24884ac4493>
—
Reply to this email directly, view it on GitHub
<#1192 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACCMXTG2TLDFXASHFFKPKL2QI5TBAVCNFSM6AAAAABVI4UL56VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNRTHE4DAMBTGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
[image: artursouza]*artursouza* left a comment (dapr/java-sdk#1192)
<#1192 (comment)>
If you run the test a few times in a row, it starts failing right at the
second run:
image.png (view on web)
<https://github.com/user-attachments/assets/1c7c2c4c-c368-471a-8f2b-f24884ac4493>
—
Reply to this email directly, view it on GitHub
<#1192 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACCMXTG2TLDFXASHFFKPKL2QI5TBAVCNFSM6AAAAABVI4UL56VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMNRTHE4DAMBTGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
- Blog: http://salaboy.com <http://salaboy.wordpress.com>
- Github user: http://github.com/salaboy
- Twitter: http://twitter.com/salaboy
- Mauricio "Salaboy" Salatino -
|
@artursouza I couldn't reproduce the issue that you showed in the screenshot.. not sure how you run the test like that (multiple times without running the testcontainers setup). But now the test is fixed. I've added some license headers to simplify the review process :) @akkie apologies.. but you need to fix the DCO here.. |
@salaboy Tried to fix the DCO issue but there are some merge conflicts during the rebase. It's not so easy to fix them from my perspective because I have no idea how they should be resolved during the rebase. As example, the actual code looks completely different to what is suggested during conflict resolution. Do you have another idea how we can fix that?
Which is strange because there is no commit: 22d9874... Add app health check support to Dapr Testcontainer (#1213) |
@akkie I am pinging you in discord |
6dab37f
to
9485b77
Compare
Let me try to fix this now. |
@akkie I have looked at the failing tests and here is what we have:
To fix it we can use something like this: /*
* Copyright 2025 The Dapr Authors
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
limitations under the License.
*/
package io.dapr.it.testcontainers;
import io.dapr.actors.ActorId;
import io.dapr.actors.client.ActorClient;
import io.dapr.actors.client.ActorProxyBuilder;
import io.dapr.actors.runtime.ActorRuntime;
import io.dapr.testcontainers.Component;
import io.dapr.testcontainers.DaprContainer;
import io.dapr.testcontainers.DaprLogLevel;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.context.SpringBootTest.WebEnvironment;
import org.springframework.test.context.DynamicPropertyRegistry;
import org.springframework.test.context.DynamicPropertySource;
import org.testcontainers.containers.Network;
import org.testcontainers.containers.wait.strategy.Wait;
import org.testcontainers.junit.jupiter.Container;
import org.testcontainers.junit.jupiter.Testcontainers;
import java.util.Map;
import java.util.Random;
import java.util.UUID;
import static org.junit.jupiter.api.Assertions.assertEquals;
@SpringBootTest(
webEnvironment = WebEnvironment.RANDOM_PORT,
classes = {
TestActorsApplication.class,
TestDaprActorsConfiguration.class
}
)
@Testcontainers
@Tag("testcontainers")
public class DaprActorsIT {
private static final Network DAPR_NETWORK = Network.newNetwork();
private static final Random RANDOM = new Random();
private static final int PORT = RANDOM.nextInt(1000) + 8000;
private static final String ACTORS_MESSAGE_PATTERN = ".*Actor API level in the cluster has been updated to 10.*";
@Container
private static final DaprContainer DAPR_CONTAINER = new DaprContainer("daprio/daprd:1.14.4")
.withAppName("actor-dapr-app")
.withNetwork(DAPR_NETWORK)
.withComponent(new Component("kvstore", "state.in-memory", "v1",
Map.of("actorStateStore", "true")))
.withDaprLogLevel(DaprLogLevel.DEBUG)
.withLogConsumer(outputFrame -> System.out.println(outputFrame.getUtf8String()))
.withAppChannelAddress("host.testcontainers.internal")
.withAppPort(PORT);
/**
* Expose the Dapr ports to the host.
*
* @param registry the dynamic property registry
*/
@DynamicPropertySource
static void daprProperties(DynamicPropertyRegistry registry) {
registry.add("dapr.http.endpoint", DAPR_CONTAINER::getHttpEndpoint);
registry.add("dapr.grpc.endpoint", DAPR_CONTAINER::getGrpcEndpoint);
registry.add("server.port", () -> PORT);
}
@Autowired
private ActorClient daprActorClient;
@Autowired
private ActorRuntime daprActorRuntime;
@BeforeEach
public void setUp(){
org.testcontainers.Testcontainers.exposeHostPorts(PORT);
daprActorRuntime.registerActor(TestActorImpl.class);
// Ensure the subscriptions are registered
Wait.forLogMessage(ACTORS_MESSAGE_PATTERN, 1).waitUntilReady(DAPR_CONTAINER);
}
@Test
public void testActors() {
ActorProxyBuilder<TestActor> builder = new ActorProxyBuilder<>(TestActor.class, daprActorClient);
ActorId actorId = ActorId.createRandom();
TestActor actor = builder.build(actorId);
String message = UUID.randomUUID().toString();
String echoedMessage = actor.echo(message);
assertEquals(echoedMessage, message);
}
} The key idea is that we use
Please give it a try and let me know. |
@artursouza @akkie is fixed now.. but the damn DCO is blocking us to ren the pipelines again.. @akkie would you mind to signoff the latest commits? |
7b544c6
to
7483cd9
Compare
@jakesmolka could you please the comments in this PR. As far as I remember you have mentioned that using Spring Boot Test with a defined port was problematic. I think we have found a way, it is not ideal, but it should work. I wonder what are your thoughts. |
Signed-off-by: Christian Kaps <[email protected]>
Signed-off-by: Christian Kaps <[email protected]>
Signed-off-by: Christian Kaps <[email protected]>
Signed-off-by: Christian Kaps <[email protected]>
Signed-off-by: Christian Kaps <[email protected]>
Signed-off-by: Christian Kaps <[email protected]>
Signed-off-by: Christian Kaps <[email protected]>
Signed-off-by: Christian Kaps <[email protected]>
Co-authored-by: Cassie Coyle <[email protected]> Signed-off-by: Christian Kaps <[email protected]>
…r#1201) Signed-off-by: Christian Kaps <[email protected]>
…rs module (dapr#1210) * feat: Adding basic HTTPEndpoint configuration support in testcontainers module Signed-off-by: Laurent Broudoux <[email protected]> * feat: dapr#1209 Adding test for HTTPEndpoint in testcontainers module Signed-off-by: Laurent Broudoux <[email protected]> --------- Signed-off-by: Laurent Broudoux <[email protected]> Signed-off-by: Christian Kaps <[email protected]>
Signed-off-by: Christian Kaps <[email protected]>
* Add app health check support to Dapr Testcontainer Signed-off-by: Artur Ciocanu <[email protected]> * Some minor cleanup Signed-off-by: Artur Ciocanu <[email protected]> * Move waiting to beforeEach, it looks more natural Signed-off-by: Artur Ciocanu <[email protected]> --------- Signed-off-by: Artur Ciocanu <[email protected]> Co-authored-by: Artur Ciocanu <[email protected]> Signed-off-by: Christian Kaps <[email protected]>
Signed-off-by: Christian Kaps <[email protected]>
* Add app health check support to Dapr Testcontainer Signed-off-by: Artur Ciocanu <[email protected]> * Some minor cleanup Signed-off-by: Artur Ciocanu <[email protected]> * Move waiting to beforeEach, it looks more natural Signed-off-by: Artur Ciocanu <[email protected]> --------- Signed-off-by: Artur Ciocanu <[email protected]> Co-authored-by: Artur Ciocanu <[email protected]> Signed-off-by: Christian Kaps <[email protected]>
Signed-off-by: Artur Souza <[email protected]> Signed-off-by: Christian Kaps <[email protected]>
* Add app health check support to Dapr Testcontainer Signed-off-by: Artur Ciocanu <[email protected]> * Some minor cleanup Signed-off-by: Artur Ciocanu <[email protected]> * Move waiting to beforeEach, it looks more natural Signed-off-by: Artur Ciocanu <[email protected]> --------- Signed-off-by: Artur Ciocanu <[email protected]> Co-authored-by: Artur Ciocanu <[email protected]> Signed-off-by: Christian Kaps <[email protected]>
Signed-off-by: Christian Kaps <[email protected]>
ace4137
to
a9fdda8
Compare
Description
This PR adds testcontainer based integration tests for actors. This is to make sure that actors really work with testcontainers.
Issue reference
This PR was crated based on a Discord discussion with @salaboy to check if actors works with testcontainers.