Fix flaky tests

[mslabek]

Describe your changes

Checklist before merge

• Related issue ID is placed at the beginning of PR title in [brackets] (can be GH issue or Nu Jira issue)
• Code is cleaned from temporary changes and commented out lines
• Parts of the code that are not easy to understand are documented in the code
• Changes are covered by automated tests
• Showcase in dev-application.conf added to demonstrate the feature
• Documentation added or updated
• Added entry in Changelog.md describing the change from the perspective of a public distribution user
• Added MigrationGuide.md entry in the appropriate subcategory if introducing a breaking change
• Verify that PR will be squashed during merge

Summary by CodeRabbit

• Bug Fixes

  • Improved error handling in integration tests to enhance reliability during asynchronous operations.
  • Updated test methods to ensure accurate deployment status after failures.

• Refactor

  • Enhanced the structure of integration tests for better clarity and reusability of failure handling logic.

• Chores

  • Updated import statements for better functionality in test cases and Kafka client operations.
  • Modified the Kafka client to improve topic creation handling with a more efficient waiting mechanism.

[github-actions]

updated: #7245
⚠️ Be careful! Snapshot changes are not necessarily the cause of the error. Check the logs.

[coderabbitai]

📝 Walkthrough

📝 Walkthrough

Walkthrough

The pull request includes modifications to two primary files: PeriodicProcessServiceIntegrationTest.scala and KafkaClient.scala.

In PeriodicProcessServiceIntegrationTest.scala, several enhancements have been made to improve test reliability and error handling. A new import statement for timing assertions has been added. The test method for waiting on schedules has been updated to utilize an eventually block, allowing for more robust handling of asynchronous operations. Additionally, a helper function for testing failed event handlers has been introduced, improving clarity and reusability. A check for deployment state after handling a failure has also been included in the rescheduling test method.

In KafkaClient.scala, the createTopic method has been modified to replace the previous retry mechanism with a blocking call using Await.result, which specifies a maximum wait time of 10 seconds for the topic to become available. This change enhances the control flow of the method while maintaining the overall structure of the class without altering method signatures or introducing new methods.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant PeriodicProcessService
    participant KafkaClient

    User->>PeriodicProcessService: Trigger deployment
    PeriodicProcessService->>KafkaClient: Create topic
    KafkaClient->>KafkaClient: Await topic availability
    KafkaClient-->>PeriodicProcessService: Topic created
    PeriodicProcessService->>User: Deployment completed

Loading

sequenceDiagram
    participant User
    participant PeriodicProcessService

    User->>PeriodicProcessService: Trigger schedule
    PeriodicProcessService->>PeriodicProcessService: Wait for previous schedule to finish
    PeriodicProcessService-->>User: Schedule deployed

Loading

---

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 21bad8d and d57db70.

📒 Files selected for processing (1)

• utils/kafka-test-utils/src/main/scala/pl/touk/nussknacker/engine/kafka/KafkaClient.scala (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• utils/kafka-test-utils/src/main/scala/pl/touk/nussknacker/engine/kafka/KafkaClient.scala

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (2)

utils/kafka-test-utils/src/main/scala/pl/touk/nussknacker/engine/kafka/KafkaClient.scala (1)

35-42: LGTM: Implementation effectively addresses flaky tests

The blocking implementation with retry logic is a good solution for ensuring topic availability in test environments. The 10-second timeout with 1-second retry intervals provides a good balance between reliability and test execution time.

Consider these minor improvements:

 val maxTime = 10.seconds
 Await.result(
   retry.Pause(10, 1.second)(Timer.default)(
     Future {
-      topic(name)
+      topic(name).getOrElse(throw new RuntimeException(s"Topic $name not available after creation"))
     }
   ),
   maxTime
 )

This would provide a more descriptive error message if the topic creation fails.

engine/flink/management/periodic/src/test/scala/pl/touk/nussknacker/engine/management/periodic/PeriodicProcessServiceIntegrationTest.scala (1)

Line range hint 432-450: Consider enhancing the eventually block with timeout configuration

The use of eventually block is a good solution for handling async operations and fixing flaky tests. However, consider adding explicit timeout and interval configurations to make the test behavior more predictable and failures more descriptive.

Consider enhancing the eventually block like this:

     val firstActivity = eventually {
+      implicit val patienceConfig = PatienceConfig(
+        timeout = Span(10, Seconds),
+        interval = Span(100, Millis)
+      )
       val result = service.getScenarioActivitiesSpecificToPeriodicProcess(processIdWithName).futureValue
       result should not be empty
       result.head.asInstanceOf[ScenarioActivity.PerformedScheduledExecution]
-    }
+    }(PatienceConfiguration.Timeout(10.seconds), PatienceConfiguration.Interval(100.milliseconds))

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 1a4c2d0 and 21bad8d.

📒 Files selected for processing (2)

• engine/flink/management/periodic/src/test/scala/pl/touk/nussknacker/engine/management/periodic/PeriodicProcessServiceIntegrationTest.scala (3 hunks)
• utils/kafka-test-utils/src/main/scala/pl/touk/nussknacker/engine/kafka/KafkaClient.scala (2 hunks)

🔇 Additional comments (2)

utils/kafka-test-utils/src/main/scala/pl/touk/nussknacker/engine/kafka/KafkaClient.scala (1)

15-15: LGTM: Import addition is appropriate

The addition of Await to the concurrent imports is necessary for the blocking implementation in createTopic.

engine/flink/management/periodic/src/test/scala/pl/touk/nussknacker/engine/management/periodic/PeriodicProcessServiceIntegrationTest.scala (1)

16-16: LGTM! Good addition for async testing

The import of timing-related classes is appropriate for configuring eventual consistency checks in async tests.