You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@streampipes.apache.org by "IsaakKrut (via GitHub)" <gi...@apache.org> on 2024/04/12 21:06:36 UTC

[PR] 2374 provide a framework to easily unit test processing elements

IsaakKrut opened a new pull request, #2730:
URL: https://github.com/apache/streampipes/pull/2730

   
   Added a couple of features to the test executor.
   
   1. Option to provide custom configuration. TestComposeProcessor required to set output strategies and TestMergeByTimeProcessor required to set output key selectors.
   2. Mocked mockParams.getModel(). Required for processors of type StreamPipesDataProcessor, which generate their own extractor based on the graph provided
   3. Updated event creation to detect the prefix in the map keys and parse it properly
   
   Refactored all tests in streampipes-processors-filters-jvm to use the executor
    
   PR introduces (a) breaking change(s): no
   
   PR introduces (a) deprecation(s): no
   


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

To unsubscribe, e-mail: dev-unsubscribe@streampipes.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test(#2374): provide a framework to easily unit test processing elements

Posted by "IsaakKrut (via GitHub)" <gi...@apache.org>.
IsaakKrut commented on PR #2730:
URL: https://github.com/apache/streampipes/pull/2730#issuecomment-2059198608

   @tenthe I created issue #2737. Could you assign it to me?


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

To unsubscribe, e-mail: dev-unsubscribe@streampipes.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test(#2374): provide a framework to easily unit test processing elements

Posted by "tenthe (via GitHub)" <gi...@apache.org>.
tenthe commented on PR #2730:
URL: https://github.com/apache/streampipes/pull/2730#issuecomment-2056556959

   Thanks a lot for the PR and updating the old tests. I really appreciate your effort in improving the testing framework step by step. 
   
   While reviewing the current version, I have some minor comments that I think could enhance the overall clarity and cleanliness of the codebase:
   
   1. Regarding the `ProcessingElementTestExecutor`, I'm not entirely convinced if adding a separate method to validate an exception is thrown is the best approach. Instead, I suggest that the test asserts that an exception is thrown. This adjustment would make the API cleaner, as it would prevent the `expectedOutputEvents` and `invocationConfigs` parameters from consistently being empty or null. What are your thoughts on this?
   
   2. Additionally, I believe we can make some improvements to the API of `ProcessingElementTestExecutor`. Currently, all methods are static. My suggestion is to make these variables non-static by passing them into the constructor. This approach would allow us to provide different constructors if certain parameters must not be set. Specifically, I propose adding `IStreamPipesDataProcessor processor`, `Map<String, Object> userConfiguration`, and `Consumer<DataProcessorInvocation> invocationConfig` as constructor parameters. Consequently, the `run` method's signature would be updated to accept `List<Map<String, Object>> inputEvents` and `List<Map<String, Object>> expectedOutputEvents`. What do you think of that?
   
   Please feel free to respond to the comments, especially if you have a different opinion.
   As soon as we have cleaned up the API a bit, we can think about how to remove the stream prefixes from the tests.  These currently make the tests somewhat difficult to read


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

To unsubscribe, e-mail: dev-unsubscribe@streampipes.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test(#2374): provide a framework to easily unit test processing elements

Posted by "tenthe (via GitHub)" <gi...@apache.org>.
tenthe merged PR #2730:
URL: https://github.com/apache/streampipes/pull/2730


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

To unsubscribe, e-mail: dev-unsubscribe@streampipes.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test(#2374): provide a framework to easily unit test processing elements

Posted by "IsaakKrut (via GitHub)" <gi...@apache.org>.
IsaakKrut commented on PR #2730:
URL: https://github.com/apache/streampipes/pull/2730#issuecomment-2057118780

   @tenthe Suggestions make sense to me. Made the updates


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

To unsubscribe, e-mail: dev-unsubscribe@streampipes.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] test(#2374): provide a framework to easily unit test processing elements

Posted by "tenthe (via GitHub)" <gi...@apache.org>.
tenthe commented on PR #2730:
URL: https://github.com/apache/streampipes/pull/2730#issuecomment-2058858161

   @IsaakKrut, thanks for the changes.
   I'll merge this PR. 
   As a next step, my suggestion would be to move the `ProcessingElementTestExecutor` into the module `streampipes-test-utils`. You can add a new package there.
   
   Furthermore, I would suggest to write a builder to create the `userConfigurations`. Within this builder we will hopefully be able to 'hide' the prefix information of event streams.
   
   @IsaakKrut, do you want to provide a suggestion for the design of such an API?
   Do you want to create an issue for this or shoud I do that?


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

To unsubscribe, e-mail: dev-unsubscribe@streampipes.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org