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

Update assertion in CSPM test to check for 'guid' in response #601

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

kooomix
Copy link
Contributor

@kooomix kooomix commented Jan 29, 2025

PR Type

Tests, Bug fix


Description

  • Updated CSPM test assertion to check for guid in response.

  • Improved validation logic for cloud account creation.


Changes walkthrough 📝

Relevant files
Tests
cspm.py
Update assertion logic in CSPM test                                           

tests_scripts/accounts/cspm.py

  • Modified assertion to check for guid in the response.
  • Replaced previous check for "Cloud account created" in response.
  • +1/-1     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • 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

    Validation Coverage

    The new assertion only checks for the presence of 'guid' key but not its value or format. Consider adding validation that the guid is non-empty and follows expected format.

    assert "guid" in res, f"guid not in {res}"

    @kooomix kooomix merged commit 58ac40d into master Jan 29, 2025
    1 check passed
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add type validation for response

    Add a type check for the response to ensure it's a dictionary before checking for
    'guid' key, preventing potential AttributeError if response is a string or other
    type.

    tests_scripts/accounts/cspm.py [205]

    -assert "guid" in res, f"guid not in {res}"
    +assert isinstance(res, dict) and "guid" in res, f"Expected dictionary with 'guid' key, got: {res}"
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: The suggestion adds important type validation to prevent potential runtime errors, as the 'in' operator could fail if 'res' is not a dictionary. This is a significant improvement for test robustness and error clarity.

    8

    Copy link

    Failed to generate code suggestions for PR

    @kooomix kooomix deleted the accountsfixes branch February 3, 2025 12:25
    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