Implement Simple Event polling and create profile based testing #200

The-Huginn · 2024-04-30T14:38:27Z

Implement simple Event polling processor, which adjust the received events and fires those events
Implement simple testing framework for manual event firing - i.e. GitHubEventEmitter - new interface for firing events, our event polling processor)
Implement another testing framework for profile aware testing, between sse and manually fired events from polling
Rewrite tests to use profile aware testing

Future work:

Tests for event firing testing framework
Implement caching of events since last polling (currently we take only the last event and fire it) + tests

xstefank · 2024-05-29T10:59:09Z

src/main/java/org/wildfly/bot/polling/EventPollingProcessor.java

+     *           https://docs.github.com/en/rest/using-the-rest-api/github-event-types?apiVersion=2022-11-28
+     *           and Events sent by GitHub SSE https://docs.github.com/en/webhooks/webhook-events-and-payloads
+     */
+    private static final Map<GHEvent, Tuple2<String, Boolean>> typeToEventMap = Map.of(


maybe it would be better from maintainability and readability point of view to not use Tuple2

what would you like to see used instead?

Custom named wrapper.

src/main/java/org/wildfly/bot/polling/EventPollingProcessor.java

xstefank · 2024-05-29T11:08:45Z

src/main/java/org/wildfly/bot/polling/GitHubEventEmitter.java

+@FunctionalInterface
+public interface GitHubEventEmitter<T extends Throwable> {
+
+    void fire() throws T;


Why do we need this since fire() method in EventPollingProcessor is scheduled?

For the testing framework, so we can manually fire the events once everything is mocked.

If it's just for testing, maybe think if you can reuse automatic polling to stay more closely aligned with the original code.

I think this single interface makes it much easier for testing, as otherwise how could we discover the correct beans?

xstefank · 2024-05-29T11:11:39Z

src/main/java/org/wildfly/bot/polling/GitHubEventPreprocessor.java

+@FunctionalInterface
+public interface GitHubEventPreprocessor {
+    GitHubEventPreprocessor INSTANCE = new DefaultGitHubEventPreprocessor();
+    ObjectMapper objectMapper = new ObjectMapper();


We should reuse objectmapper if possible.

it is static by default this way - in the interface. It's for this reason in the interface, so implementing classes can use it, such as PushEventPreprocessor does.

maybe then don't use fields in interfaces

I believe this is a common practice, for reusing common objects or for providing default static implementation

src/test/java/org/wildfly/bot/utils/testing/model/GitHubEventPayload.java

xstefank · 2024-05-29T13:58:01Z

src/test/java/org/wildfly/bot/webhooks/PRAutomaticLabelingTest.java

+                PullRequestGitHubEventPayload::new);
+
+        pullRequestJson = TestModel
+                .setPullRequestJsonBuilderBuild(pullRequestJsonBuilder -> pullRequestJsonBuilder.action(SYNCHRONIZE));


setPullRequestJson

Sure, but note, it's rather the builder we provide there.

and you called it BuilderBuild? setPRJSonBuilder then

xstefank · 2024-05-30T06:12:28Z

src/test/java/org/wildfly/bot/webhooks/PRChecksTest.java

+        TestModel.given(mocks -> WildflyGitHubBotTesting.mockRepo(mocks, wildflyConfigFile, pullRequestJson))
+                .sseEventOptions(eventSenderOptions -> eventSenderOptions.payloadFromString(pullRequestJson.jsonString())
+                        .event(GHEvent.PULL_REQUEST))
+                .pollingEventOptions(eventSenderOptions -> eventSenderOptions.eventFromPayload(pullRequestJson.jsonString()))


maybe we don't need to repeat it if it's same for sse and polling?

Sure, I can add some default methods. I was just worried to not hide too many details from the user, such that there would be too much unnecessary "magic" happening in the testing framework.

magic is what we do to make the end user code easier

This has been already done previously, please see here. Let me know, if it fits the needs

src/test/java/org/wildfly/bot/webhooks/PRCommitCheckTest.java

xstefank · 2024-05-30T06:21:21Z

src/test/java/org/wildfly/bot/webhooks/PRConfigFileChangeTest.java

+                () -> SsePullRequestPayload.builder(TestConstants.VALID_PR_TEMPLATE_JSON),
+                PullRequestGitHubEventPayload::new);
+
+        pullRequestJson = TestModel.getPullRequestJson();


Just when I see this side to side, do you think that this new API is easier?

Sure, I will provide more default methods. Also, after this PR I would like to open issue, about prettyfying the old school builder. I don't think for example we need to provide TestConstants.VALID_PR_TEMPLATE_JSON all the time, etc. Similar to what I did with PullRequestGitHubEventPayload.

…lder

…aware tests

…cations

xstefank · 2024-06-26T09:30:23Z

src/test/java/org/wildfly/bot/webhooks/PRAppendingMessageTest.java

-                .then().github(mocks -> {
+
+        TestModel.given(
+                mocks -> WildflyGitHubBotTesting.mockRepo(mocks, wildflyConfigFile, pullRequestJson, mockedContext))


We can probably simplify this later. Not in this PR.

xstefank · 2024-06-26T10:55:28Z

src/test/java/org/wildfly/bot/webhooks/PRReviewFixMeLabelTest.java

+                .sseEventOptions(eventSenderOptions -> eventSenderOptions.payloadFromString(pullRequestReviewJson.jsonString())
+                        .event(GHEvent.PULL_REQUEST_REVIEW))
+                .pollingEventOptions(
+                        eventSenderOptions -> eventSenderOptions.eventFromPayload(pullRequestReviewJson.jsonString()))


This could be simplified now right?

The-Huginn requested a review from xstefank April 30, 2024 14:38

The-Huginn force-pushed the event-polling branch 2 times, most recently from 7aec6ee to 106cd6b Compare May 2, 2024 06:49

The-Huginn requested a review from petrberan May 2, 2024 10:34

xstefank requested changes May 30, 2024

View reviewed changes

The-Huginn force-pushed the event-polling branch from 8f5ede7 to 5edb810 Compare June 3, 2024 11:50

The-Huginn requested a review from xstefank June 3, 2024 11:55

The-Huginn added 9 commits June 3, 2024 14:01

organize into new packages

2b16a83

create simple testing framework for EventPollingBeans

17b037d

create simple testing tooling

bff6038

rename and unify usernames in tests

21ba5ad

Create Interface for manipulation with the Json PullRequest and a bui…

723d00f

…lder

unify jsons used for testing

31fdb2a

Update interfaces for common functionality of json manipulation

d496791

Implement PollingProcessor and create testing framework with profile …

09c6820

…aware tests

Update tests to be aware of testing profile

118e6cf

The-Huginn force-pushed the event-polling branch from 5edb810 to ff7fe48 Compare June 3, 2024 12:08

The-Huginn force-pushed the event-polling branch from ff7fe48 to 2a5c507 Compare June 25, 2024 08:33

Remove ThenTest, provide default methods, wrap reflection method invo…

63d72ae

…cations

The-Huginn force-pushed the event-polling branch from 2a5c507 to 63d72ae Compare June 25, 2024 08:45

xstefank approved these changes Jun 26, 2024

View reviewed changes

xstefank merged commit ab5aabe into wildfly:main Jun 26, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Simple Event polling and create profile based testing #200

Implement Simple Event polling and create profile based testing #200

The-Huginn commented Apr 30, 2024

xstefank May 29, 2024

The-Huginn Jun 3, 2024

xstefank Jun 13, 2024

xstefank May 29, 2024

The-Huginn Jun 3, 2024

xstefank Jun 13, 2024

The-Huginn Jun 25, 2024

xstefank May 29, 2024

The-Huginn Jun 3, 2024

xstefank Jun 13, 2024

The-Huginn Jun 25, 2024

xstefank May 29, 2024

The-Huginn Jun 3, 2024

xstefank Jun 14, 2024

xstefank May 30, 2024

The-Huginn Jun 3, 2024

xstefank Jun 14, 2024

The-Huginn Jun 26, 2024

xstefank May 30, 2024

The-Huginn Jun 3, 2024

xstefank Jun 26, 2024

xstefank Jun 26, 2024

Implement Simple Event polling and create profile based testing #200

Implement Simple Event polling and create profile based testing #200

Conversation

The-Huginn commented Apr 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment