You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2020/02/11 15:38:15 UTC
[GitHub] [flink-statefun] sjwiesman opened a new pull request #22:
[FLINK-16002][statefun][testutil] Add stateful functions test utilities
sjwiesman opened a new pull request #22: [FLINK-16002][statefun][testutil] Add stateful functions test utilities
URL: https://github.com/apache/flink-statefun/pull/22
The stateful functions SDK does not have any dependencies on the Flink runtime. These test utilities provide a minimal synchronous runtime implementation to facilitate simple unit testing.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink-statefun] sjwiesman edited a comment on issue #22:
[FLINK-16002][statefun][testutil] Add stateful functions test utilities
Posted by GitBox <gi...@apache.org>.
sjwiesman edited a comment on issue #22: [FLINK-16002][statefun][testutil] Add stateful functions test utilities
URL: https://github.com/apache/flink-statefun/pull/22#issuecomment-587144546
Gordon, this package game out of a need to unit test the functions for my statefun workshop. If it helps, you can see this branch for real world usage
https://github.com/sjwiesman/flink-statefun/tree/workshop?files=1
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink-statefun] sjwiesman commented on issue #22:
[FLINK-16002][statefun][testutil] Add stateful functions test utilities
Posted by GitBox <gi...@apache.org>.
sjwiesman commented on issue #22: [FLINK-16002][statefun][testutil] Add stateful functions test utilities
URL: https://github.com/apache/flink-statefun/pull/22#issuecomment-584697205
cc @igalshilman
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink-statefun] tzulitai commented on a change in pull request
#22: [FLINK-16002][statefun][testutil] Add stateful functions test utilities
Posted by GitBox <gi...@apache.org>.
tzulitai commented on a change in pull request #22: [FLINK-16002][statefun][testutil] Add stateful functions test utilities
URL: https://github.com/apache/flink-statefun/pull/22#discussion_r382448428
##########
File path: statefun-testutil/src/main/java/org/apache/flink/statefun/testutils/function/TestContext.java
##########
@@ -0,0 +1,198 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 org.apache.flink.statefun.testutils.function;
+
+import java.time.Duration;
+import java.time.Instant;
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.PriorityQueue;
+import java.util.Queue;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+import org.apache.flink.statefun.sdk.Address;
+import org.apache.flink.statefun.sdk.AsyncOperationResult;
+import org.apache.flink.statefun.sdk.Context;
+import org.apache.flink.statefun.sdk.StatefulFunction;
+import org.apache.flink.statefun.sdk.io.EgressIdentifier;
+
+/** A simple context that is strictly synchronous and captures all responses. */
+class TestContext implements Context {
+
+ private final Address selfAddress;
+
+ private final StatefulFunction function;
+
+ private final Map<EgressIdentifier<?>, List<Object>> outputs;
+
+ private final Queue<Envelope> messages;
+
+ private final PriorityQueue<PendingMessage> pendingMessage;
+
+ private Map<Address, List<Object>> responses;
+
+ private Address from;
+
+ private long watermark;
+
+ TestContext(Address selfAddress, StatefulFunction function, Instant startTime) {
+ this.selfAddress = Objects.requireNonNull(selfAddress);
+ this.function = function;
+
+ this.watermark = startTime.toEpochMilli();
+ this.messages = new ArrayDeque<>();
+ this.pendingMessage = new PriorityQueue<>(Comparator.comparingLong(a -> a.timer));
+ this.outputs = new HashMap<>();
+ }
+
+ @Override
+ public Address self() {
+ return selfAddress;
+ }
+
+ @Override
+ public Address caller() {
+ return from;
+ }
+
+ @Override
+ public void reply(Object message) {
+ Address to = caller();
+ if (to == null) {
+ throw new IllegalStateException("The caller address is null");
+ }
+
+ send(to, message);
+ }
+
+ @Override
+ public void send(Address to, Object message) {
+ if (to.equals(selfAddress)) {
+ messages.add(new Envelope(self(), to, message));
+ return;
+ }
+ responses.computeIfAbsent(to, ignore -> new ArrayList<>()).add(message);
+ }
+
+ @Override
+ public <T> void send(EgressIdentifier<T> egress, T message) {
+ outputs.computeIfAbsent(egress, ignore -> new ArrayList<>()).add(message);
+ }
+
+ @Override
+ public void sendAfter(Duration delay, Address to, Object message) {
+ pendingMessage.add(
+ new PendingMessage(new Envelope(self(), to, message), watermark + delay.toMillis()));
+ }
+
+ @Override
+ public <M, T> void registerAsyncOperation(M metadata, CompletableFuture<T> future) {
+ T value = null;
+ Throwable error = null;
+
+ try {
+ value = future.get();
+ } catch (InterruptedException e) {
+ throw new RuntimeException("Failed to get results from async action", e);
+ } catch (ExecutionException e) {
+ error = e.getCause();
+ }
+
+ AsyncOperationResult.Status status;
+ if (error == null) {
+ status = AsyncOperationResult.Status.SUCCESS;
+ } else {
+ status = AsyncOperationResult.Status.FAILURE;
+ }
+
+ AsyncOperationResult<M, T> result = new AsyncOperationResult<>(metadata, status, value, error);
+ messages.add(new Envelope(self(), self(), result));
+ }
+
+ @SuppressWarnings("unchecked")
+ <T> List<T> getEgress(EgressIdentifier<T> identifier) {
+ List<?> values = outputs.getOrDefault(identifier, Collections.emptyList());
+
+ // Because the type is part of the identifier key
+ // this cast is always safe.
+ return (List<T>) values;
+ }
+
+ Map<Address, List<Object>> invoke(Address from, Object message) {
+ messages.add(new Envelope(from, null, message));
+
+ return processAllMessages();
+ }
+
+ Map<Address, List<Object>> tick(Duration duration) {
+ watermark += duration.toMillis();
+
+ while (!pendingMessage.isEmpty() && pendingMessage.peek().timer <= watermark) {
+ messages.add(pendingMessage.poll().envelope);
+ }
+
+ return processAllMessages();
+ }
+
+ private Map<Address, List<Object>> processAllMessages() {
Review comment:
mostly a nitpick:
I feel like the responsibility of this method should be separated from the `TestContext`, perhaps a `SynchronousReductions` class, which uses the `TestContext`.
Adding to message queue and draining it will reside in the `SynchronousReductions`.
I think it's good as it it is though, so will still merge this as it. Just food for thought.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink-statefun] sjwiesman commented on a change in pull request
#22: [FLINK-16002][statefun][testutil] Add stateful functions test utilities
Posted by GitBox <gi...@apache.org>.
sjwiesman commented on a change in pull request #22: [FLINK-16002][statefun][testutil] Add stateful functions test utilities
URL: https://github.com/apache/flink-statefun/pull/22#discussion_r378342377
##########
File path: statefun-testutil/pom.xml
##########
@@ -0,0 +1,50 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you 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.
+-->
+<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xmlns="http://maven.apache.org/POM/4.0.0"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <parent>
+ <artifactId>statefun-parent</artifactId>
+ <groupId>org.apache.flink</groupId>
+ <version>1.1-SNAPSHOT</version>
+ </parent>
+ <modelVersion>4.0.0</modelVersion>
+
+ <artifactId>statefun-testutil</artifactId>
+
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.flink</groupId>
+ <artifactId>statefun-sdk</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.hamcrest</groupId>
+ <artifactId>hamcrest-all</artifactId>
+ <version>1.3</version>
+ </dependency>
+ <dependency>
Review comment:
done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink-statefun] igalshilman commented on issue #22:
[FLINK-16002][statefun][testutil] Add stateful functions test utilities
Posted by GitBox <gi...@apache.org>.
igalshilman commented on issue #22: [FLINK-16002][statefun][testutil] Add stateful functions test utilities
URL: https://github.com/apache/flink-statefun/pull/22#issuecomment-586996294
We had a discussion with Seth offline, and I definitely think there is need for this test kit.
To summarize the discussion:
There are these two ways of approaching unit testing this (probably more)
a) would be just call the stateful function with some TestContext and make assertions on the side effects.
Something a long the lines of
```
assertThat(functionUnderTest.invoke(context, message), allOf(
hasOutgoingMessage( messageMatches(...) )
hasOutgoingEgress( ... )
hasDelayedMessages( ...)
);
```
b) actually simulate some stuff happening, and later collecting the results
and asserting on the expected behaviour.
This PR leans towards (b) and I can definitely see the value of that after seeing some examples, so I'm +1 to bring this to shape and merge this, I also like the use of matchers they make the tests more readable!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink-statefun] igalshilman commented on a change in pull request
#22: [FLINK-16002][statefun][testutil] Add stateful functions test utilities
Posted by GitBox <gi...@apache.org>.
igalshilman commented on a change in pull request #22: [FLINK-16002][statefun][testutil] Add stateful functions test utilities
URL: https://github.com/apache/flink-statefun/pull/22#discussion_r378255439
##########
File path: statefun-testutil/pom.xml
##########
@@ -0,0 +1,50 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you 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.
+-->
+<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xmlns="http://maven.apache.org/POM/4.0.0"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <parent>
+ <artifactId>statefun-parent</artifactId>
+ <groupId>org.apache.flink</groupId>
+ <version>1.1-SNAPSHOT</version>
+ </parent>
+ <modelVersion>4.0.0</modelVersion>
+
+ <artifactId>statefun-testutil</artifactId>
+
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.flink</groupId>
+ <artifactId>statefun-sdk</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.hamcrest</groupId>
+ <artifactId>hamcrest-all</artifactId>
+ <version>1.3</version>
+ </dependency>
+ <dependency>
Review comment:
well, this entire artifact is "test dependency", so I think it needs to be in scope compile.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink-statefun] sjwiesman commented on a change in pull request
#22: [FLINK-16002][statefun][testutil] Add stateful functions test utilities
Posted by GitBox <gi...@apache.org>.
sjwiesman commented on a change in pull request #22: [FLINK-16002][statefun][testutil] Add stateful functions test utilities
URL: https://github.com/apache/flink-statefun/pull/22#discussion_r377829187
##########
File path: statefun-testutil/pom.xml
##########
@@ -0,0 +1,50 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you 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.
+-->
+<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xmlns="http://maven.apache.org/POM/4.0.0"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <parent>
+ <artifactId>statefun-parent</artifactId>
+ <groupId>org.apache.flink</groupId>
+ <version>1.1-SNAPSHOT</version>
+ </parent>
+ <modelVersion>4.0.0</modelVersion>
+
+ <artifactId>statefun-testutil</artifactId>
+
+
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.flink</groupId>
+ <artifactId>statefun-sdk</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.hamcrest</groupId>
+ <artifactId>hamcrest-all</artifactId>
+ <version>1.3</version>
+ </dependency>
+ <dependency>
Review comment:
Do you think we should actually bundle this or mark it as a compile dependency?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink-statefun] tzulitai closed pull request #22:
[FLINK-16002][statefun][testutil] Add stateful functions test utilities
Posted by GitBox <gi...@apache.org>.
tzulitai closed pull request #22: [FLINK-16002][statefun][testutil] Add stateful functions test utilities
URL: https://github.com/apache/flink-statefun/pull/22
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink-statefun] tzulitai commented on a change in pull request
#22: [FLINK-16002][statefun][testutil] Add stateful functions test utilities
Posted by GitBox <gi...@apache.org>.
tzulitai commented on a change in pull request #22: [FLINK-16002][statefun][testutil] Add stateful functions test utilities
URL: https://github.com/apache/flink-statefun/pull/22#discussion_r381892048
##########
File path: statefun-testutil/pom.xml
##########
@@ -0,0 +1,51 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you 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.
+-->
+<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xmlns="http://maven.apache.org/POM/4.0.0"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <parent>
+ <artifactId>statefun-parent</artifactId>
+ <groupId>org.apache.flink</groupId>
+ <version>1.1-SNAPSHOT</version>
+ </parent>
+ <modelVersion>4.0.0</modelVersion>
+
+ <artifactId>statefun-testutil</artifactId>
+
Review comment:
nit: extra redundant line
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink-statefun] sjwiesman commented on issue #22:
[FLINK-16002][statefun][testutil] Add stateful functions test utilities
Posted by GitBox <gi...@apache.org>.
sjwiesman commented on issue #22: [FLINK-16002][statefun][testutil] Add stateful functions test utilities
URL: https://github.com/apache/flink-statefun/pull/22#issuecomment-587144546
Gordon, this package game out of a need to unit test the functions for my statefun workshop. If it helps, you can see the usage on on this branch for real world usage
https://github.com/sjwiesman/flink-statefun/tree/workshop?files=1
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [flink-statefun] tzulitai commented on a change in pull request
#22: [FLINK-16002][statefun][testutil] Add stateful functions test utilities
Posted by GitBox <gi...@apache.org>.
tzulitai commented on a change in pull request #22: [FLINK-16002][statefun][testutil] Add stateful functions test utilities
URL: https://github.com/apache/flink-statefun/pull/22#discussion_r381892242
##########
File path: statefun-testutil/src/main/java/org/apache/flink/statefun/testutils/function/TestContext.java
##########
@@ -0,0 +1,198 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 org.apache.flink.statefun.testutils.function;
+
+import java.time.Duration;
+import java.time.Instant;
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.PriorityQueue;
+import java.util.Queue;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+import org.apache.flink.statefun.sdk.Address;
+import org.apache.flink.statefun.sdk.AsyncOperationResult;
+import org.apache.flink.statefun.sdk.Context;
+import org.apache.flink.statefun.sdk.StatefulFunction;
+import org.apache.flink.statefun.sdk.io.EgressIdentifier;
+
+/** A simple context that is strictly synchronous and captures all responses. */
+class TestContext implements Context {
+
+ private final Address selfAddress;
+
+ private final StatefulFunction function;
+
+ private final Map<EgressIdentifier<?>, List<Object>> outputs;
+
+ private final Queue<Envelope> messages;
+
+ private final PriorityQueue<PendingMessage> pendingMessage;
+
+ private Map<Address, List<Object>> responses;
+
+ private Address from;
+
+ private long watermark;
+
+ TestContext(Address selfAddress, StatefulFunction function, Instant startTime) {
+ this.selfAddress = Objects.requireNonNull(selfAddress);
+ this.function = function;
Review comment:
missing preconditions null check
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services