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

complete workflows tests #491

Merged
merged 6 commits into from
Dec 1, 2024
Merged

complete workflows tests #491

merged 6 commits into from
Dec 1, 2024

Conversation

jnathangreeg
Copy link
Contributor

@jnathangreeg jnathangreeg commented Nov 24, 2024

PR Type

Tests, Enhancement


Description

  • Added comprehensive tests for Slack, Teams, and Jira notification workflows.
  • Implemented CRUD operations for workflows and webhooks in the backend API.
  • Enhanced workflow configurations and added utility functions for message retrieval.
  • Updated system test mappings to include new workflow tests.
  • Added deployment configurations for HTTP services used in tests.

Changes walkthrough 📝

Relevant files
Tests
5 files
workflows_tests.py
Enhance and update workflow test configurations                   

configurations/system/tests_cases/workflows_tests.py

  • Added new utility functions for Slack and Teams notifications.
  • Updated test configurations for Slack and Teams workflows.
  • Renamed methods for clarity in workflow tests.
  • +43/-10 
    conf_workflows.py
    Implement workflow configuration tests                                     

    tests_scripts/workflows/conf_workflows.py

  • Implemented workflow configuration tests.
  • Added methods for creating, updating, and deleting workflows.
  • +206/-0 
    jira_workflows.py
    Add Jira notification workflow tests                                         

    tests_scripts/workflows/jira_workflows.py

  • Added Jira notification workflow tests.
  • Implemented methods for Jira workflow creation and validation.
  • +325/-0 
    slack_workflows.py
    Add Slack notification workflow tests                                       

    tests_scripts/workflows/slack_workflows.py

  • Added Slack notification workflow tests.
  • Implemented methods for Slack workflow creation and validation.
  • +330/-0 
    teams_workflows.py
    Add Teams notification workflow tests                                       

    tests_scripts/workflows/teams_workflows.py

  • Added Teams notification workflow tests.
  • Implemented methods for Teams workflow creation and validation.
  • +332/-0 
    Enhancement
    3 files
    backend_api.py
    Add API endpoints and operations for workflows and webhooks

    infrastructure/backend_api.py

  • Added API endpoints for workflows and webhooks.
  • Implemented CRUD operations for workflows and webhooks.
  • +115/-2 
    utils.py
    Add utility functions for Slack and Teams                               

    tests_scripts/workflows/utils.py

  • Added utility functions for Slack and Teams message retrieval.
  • Defined constants for workflow tests.
  • +105/-0 
    workflows.py
    Refactor workflow base class and remove redundancies         

    tests_scripts/workflows/workflows.py

  • Refactored workflow base class.
  • Removed redundant Slack and Teams workflow classes.
  • +4/-55   
    Configuration changes
    6 files
    statics.py
    Define default paths for workflow notifications                   

    systest_utils/statics.py

    • Added default paths for workflow notifications.
    +4/-0     
    apache-http.yaml
    Add HTTP service deployment configuration                               

    configurations/workflows_notifications/deployments/http/apache-http.yaml

    • Added deployment configuration for HTTP service.
    +22/-0   
    apache1-http.yaml
    Add alternative HTTP service deployment configuration       

    configurations/workflows_notifications/deployments/http1/apache1-http.yaml

    • Added alternative HTTP service deployment configuration.
    +22/-0   
    slack-alert-channel.json
    Add Slack alert channel configuration                                       

    configurations/workflows_notifications/slack-alert-channel.json

    • Added Slack alert channel configuration.
    +51/-0   
    teams-alert-channel.json
    Add Teams alert channel configuration                                       

    configurations/workflows_notifications/teams-alert-channel.json

    • Added Teams alert channel configuration.
    +52/-0   
    system_test_mapping.json
    Update test mappings for workflow tests                                   

    system_test_mapping.json

  • Updated test mappings for new workflow tests.
  • Added entries for Jira, Slack, and Teams workflows.
  • +32/-5   

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

    Signed-off-by: jnathangreeg <[email protected]>
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 PR contains tests
    🔒 Security concerns

    Sensitive information exposure:
    The code contains multiple environment variables for API tokens, webhooks and channel IDs that are logged with basic masking. Consider using a more secure secret management approach and avoid logging sensitive values even when masked.

    ⚡ Recommended focus areas for review

    Incomplete Implementation
    The Jira workflow implementation is incomplete with TODO comments for message assertion functionality that needs to be implemented

    Code Duplication
    Significant code duplication between Slack, Teams and Jira workflow implementations. Consider extracting common functionality into shared base class

    Potential Bug
    The delete_webhook method passes 'body' parameter which is not consistent with other methods using 'json'. This may cause request formatting issues

    Copy link

    codiumai-pr-agent-free bot commented Nov 24, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Fix incorrect super() call that breaks proper class initialization

    The super() call in init is incorrect - it calls Workflows directly instead of
    using super().init. This breaks proper inheritance chain initialization.

    tests_scripts/workflows/teams_workflows.py [28-29]

    -super(Workflows, self).__init__(test_driver=test_driver, test_obj=test_obj, backend=backend,
    -                                         kubernetes_obj=kubernetes_obj)
    +super().__init__(test_driver=test_driver, test_obj=test_obj, backend=backend,
    +               kubernetes_obj=kubernetes_obj)
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: The incorrect super() call can break the inheritance chain and cause initialization issues, potentially affecting the entire class functionality. This is a critical fix for proper object initialization.

    9
    Fix incorrect parameter name in HTTP DELETE request that would prevent proper JSON payload transmission

    The delete_webhook method incorrectly uses 'body' parameter in the requests.delete
    call instead of 'json'. This will cause the request to fail since the requests
    library expects 'json' for JSON payloads.

    infrastructure/backend_api.py [2994-2997]

     def delete_webhook(self, body):
         url = API_WEBHOOKS 
         params = {"customerGUID": self.selected_tenant_id}
    -    r = self.delete(url, params=params, body=body)
    +    r = self.delete(url, params=params, json=body)
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Critical fix for a bug that would prevent webhook deletion from working. The requests library expects 'json' parameter for JSON payloads, not 'body', which would cause API requests to fail.

    9
    ✅ Remove incorrect cleanup() call that causes premature resource cleanup
    Suggestion Impact:The cleanup() call inside the loop was removed, addressing the issue of premature resource cleanup as suggested.

    code diff:

    @@ -191,7 +193,7 @@
             workflows = self.backend.get_workflows()["response"]
             for workflow in workflows:
                 assert workflow["guid"] != workflow_guid, f"Expected workflow with guid {workflow_guid} to be deleted, but it still exists"
    -            self.cleanup()
    +

    The delete_and_assert_workflow method incorrectly calls cleanup() inside the loop,
    which can lead to premature cleanup and resource leaks.

    tests_scripts/workflows/conf_workflows.py [189-194]

     def delete_and_assert_workflow(self, workflow_guid):
         workflow_delete_res = self.backend.delete_workflow(workflow_guid)
         assert workflow_delete_res == "Workflow deleted", f"Expected 'Workflow deleted', but got {workflow_delete_res['response']}"
         workflows = self.backend.get_workflows()["response"]
         for workflow in workflows:
             assert workflow["guid"] != workflow_guid, f"Expected workflow with guid {workflow_guid} to be deleted, but it still exists"
    -        self.cleanup()
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Calling cleanup() inside the assertion loop can cause premature resource cleanup and potential resource leaks. This is a significant bug that could affect test reliability and resource management.

    8
    General
    Add error handling for Slack API calls to prevent test failures due to API issues

    The get_messages_from_slack_channel function should handle potential API errors by
    catching exceptions, as the Slack WebClient can raise various exceptions that could
    crash the test.

    tests_scripts/workflows/utils.py [73-77]

     def get_messages_from_slack_channel(before_test):
         formatted_time = format(before_test, ".6f")
         Logger.logger.info('Attempting to read messages from slack before timestamp ' + formatted_time)
    -    client = WebClient(token=get_env("SLACK_SYSTEM_TEST_TOKEN"))
    -    result = client.conversations_history(channel=f'{get_env("SLACK_CHANNEL_ID")}', oldest=formatted_time)
    +    try:
    +        client = WebClient(token=get_env("SLACK_SYSTEM_TEST_TOKEN"))
    +        result = client.conversations_history(channel=f'{get_env("SLACK_CHANNEL_ID")}', oldest=formatted_time)
    +    except Exception as e:
    +        Logger.logger.error(f"Error fetching Slack messages: {str(e)}")
    +        return []
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Important defensive programming addition that prevents test failures due to Slack API issues, improving test reliability and debugging.

    7
    Remove unreachable code after return statement to improve code clarity

    The delete method contains a trailing line after the return statement that creates
    unreachable code and could lead to confusion.

    infrastructure/backend_api.py [1976-1980]

    • Apply this suggestion
    Suggestion importance[1-10]: 3

    Why: Minor code cleanup suggestion to remove a trailing empty line after a return statement that could cause confusion but doesn't affect functionality.

    3

    💡 Need additional feedback ? start a PR chat

    Copy link

    Failed to generate code suggestions for PR

    tests_scripts/workflows/jira_workflows.py Outdated Show resolved Hide resolved
    tests_scripts/workflows/jira_workflows.py Outdated Show resolved Hide resolved
    tests_scripts/workflows/jira_workflows.py Show resolved Hide resolved
    tests_scripts/workflows/jira_workflows.py Show resolved Hide resolved
    system_test_mapping.json Outdated Show resolved Hide resolved
    system_test_mapping.json Outdated Show resolved Hide resolved
    Signed-off-by: jnathangreeg <[email protected]>
    @jnathangreeg jnathangreeg merged commit 4fe2cbc into master Dec 1, 2024
    3 checks passed
    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.

    2 participants