Skip to content
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

Increase attempts and sleep time for message assertion in teams workf… #540

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

kooomix
Copy link
Contributor

@kooomix kooomix commented Dec 17, 2024

User description

…lows


PR Type

Tests


Description

  • Improved the reliability of Teams workflow tests by increasing the retry mechanism parameters:
    • Increased number of attempts to 50 (from default)
    • Increased sleep time between attempts to 10 seconds (from default)
  • This change allows more time for message assertions to complete successfully, reducing potential flaky tests

Changes walkthrough 📝

Relevant files
Enhancement
teams_workflows.py
Enhance message assertion retry mechanism in Teams workflow

tests_scripts/workflows/teams_workflows.py

  • Modified assert_messages_sent call by adding parameters: attempts=50
    and sleep_time=10
  • Increased retry attempts and delay between attempts for message
    assertion verification
  • +1/-1     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @kooomix kooomix merged commit 8bb3c43 into master Dec 17, 2024
    2 checks passed
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Test Reliability
    While increasing retry attempts and sleep time may help with flaky tests, it could mask underlying timing/race condition issues that should be investigated and fixed properly

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Add a timeout parameter to prevent excessive wait times in test execution

    Consider adding a timeout mechanism to prevent indefinite waiting in case of system
    failures. With 50 attempts and 10 seconds sleep time, the total wait time could be
    up to 500 seconds.

    tests_scripts/workflows/teams_workflows.py [116]

    -self.assert_messages_sent(before_test_message_ts, self.cluster, attempts=50, sleep_time=10)
    +self.assert_messages_sent(before_test_message_ts, self.cluster, attempts=50, sleep_time=10, timeout=300)
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Adding a timeout mechanism is a valuable safeguard against test hangs and system failures, especially given the long potential wait time of 500 seconds. This would improve test reliability and prevent resource waste.

    7

    Copy link

    Failed to generate code suggestions for PR

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant