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

[DX-1780]Generate test for tyk gateway swagger #6827

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

yurisasuke
Copy link
Member

@yurisasuke yurisasuke commented Jan 14, 2025

DX-1780
Summary Create contract tests for the tyk gateway
Type Story Story
Status In Progress
Points N/A
Labels -

User description

DX-1780

Description

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning why it's required
  • I would like a code coverage CI quality gate exception and have explained why

PR Type

Tests, Enhancement


Description

  • Added a pre-request script for Tyk Gateway reload.

  • Configured Portman CLI for OpenAPI specification testing.

  • Introduced test cases for API and policy management.

  • Updated Swagger schema to allow empty string values.


Changes walkthrough 📝

Relevant files
Enhancement
prescript.js
Add pre-request script for Tyk Gateway reload                       

ci/tests/schema/specs/config/prescript.js

  • Added a script to reload Tyk Gateway using API key.
  • Included error handling for missing API key.
  • Logged responses from the Tyk Gateway reload endpoint.
  • +27/-0   
    swagger.yml
    Update Swagger schema to allow empty strings                         

    swagger.yml

  • Allowed empty string values for execution mode.
  • Updated version enum to include empty string.
  • +2/-1     
    Configuration changes
    Taskfile.yml
    Configure tasks for OpenAPI specification tests                   

    ci/tests/schema/specs/Taskfile.yml

  • Added tasks for running OpenAPI specification tests.
  • Configured Venom and npm commands for testing.
  • +9/-0     
    portman-cli-options.json
    Add Portman CLI configuration for testing                               

    ci/tests/schema/specs/config/portman-cli-options.json

  • Configured Portman CLI options for OpenAPI testing.
  • Enabled test inclusion and Newman execution.
  • Set up environment file and output paths.
  • +12/-0   
    portmanconfig.json
    Define Portman configuration for OpenAPI tests                     

    ci/tests/schema/specs/config/portmanconfig.json

  • Defined contract tests for OpenAPI operations.
  • Added variable assignments for API operations.
  • Configured pre-request scripts and request overwrites.
  • Included global settings for test execution order.
  • +144/-0 
    Dependencies
    package.json
    Add Portman dependency and npm script                                       

    ci/tests/schema/specs/package.json

  • Added Portman dependency for OpenAPI testing.
  • Configured npm start script for Portman execution.
  • +16/-0   
    Tests
    populate_gateway_test_data.yaml
    Add test cases for API and Gateway management                       

    ci/tests/schema/specs/testdata/populate_gateway_test_data.yaml

  • Added test cases for API creation and deletion.
  • Included a test case for Tyk Gateway reload.
  • Configured assertions for HTTP responses.
  • +61/-0   

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

    @yurisasuke yurisasuke requested a review from a team as a code owner January 14, 2025 13:30
    @buger
    Copy link
    Member

    buger commented Jan 14, 2025

    A JIRA Issue ID is missing from your branch name, PR title and PR description! 🦄

    Your branch: generate-test-for-tyk-gateway-open-api-spec-improved

    Your PR title: add swagger file empty string

    Your PR description:

    Description

    Related Issue

    Motivation and Context

    How This Has Been Tested

    Screenshots (if appropriate)

    Types of changes

    • Bug fix (non-breaking change which fixes an issue)
    • New feature (non-breaking change which adds functionality)
    • Breaking change (fix or feature that would cause existing functionality to change)
    • Refactoring or add test (improvements in base code or adds test coverage to functionality)

    Checklist

    • I ensured that the documentation is up to date
    • I explained why this PR updates go.mod in detail with reasoning why it's required
    • I would like a code coverage CI quality gate exception and have explained why

    If this is your first time contributing to this repository - welcome!


    Please refer to jira-lint to get started.

    Without the JIRA Issue ID in your branch name you would lose out on automatic updates to JIRA via SCM; some GitHub status checks might fail.

    Valid sample branch names:

    ‣ feature/shiny-new-feature--mojo-10'
    ‣ 'chore/changelogUpdate_mojo-123'
    ‣ 'bugfix/fix-some-strange-bug_GAL-2345'

    @yurisasuke yurisasuke changed the title add swagger file empty string [DX-1780]Generate test for tyk gateway swagger Jan 14, 2025
    Copy link
    Contributor

    github-actions bot commented Jan 14, 2025

    Swagger Changes

         _        __  __
        nullable: true
       _| |_   _ / _|/ _|  between swagger-prev.yml
      + one list entry added:
      + one map entry added:
      - one list entry removed:     + one list entry added:
      ± value change
     / _' | | | | |_| |_       and swagger-current.yml
     \__,_|\__, |_| |_|   returned four differences
    components.schemas.GraphQLConfig.properties.execution_mode.enum
    components.schemas.GraphQLConfig.properties.version.enum
    components.schemas.RateLimitSmoothing
    paths./tyk/certs/{certID}.get.tags.0
    | (_| | |_| |  _|  _|

    Copy link
    Contributor

    github-actions bot commented Jan 14, 2025

    API Changes

    no api changes detected

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Error Handling

    The error handling in the pm.sendRequest callback only logs the error but does not provide a mechanism to retry or handle failures gracefully. Consider adding retry logic or alternative handling for failed requests.

    }, function (err, res) {
      if (err) {
        console.error('Error reloading Tyk Gateway:', err);
      } else {
        console.log('Tyk Gateway reload response:', res.status);
      }
    });
    Hardcoded API ID

    The api_id in the Create API test case is hardcoded. This could lead to conflicts or issues in environments where this ID is already in use. Consider dynamically generating or parameterizing the api_id.

    "api_id": "f84ve1a04e5648c2797170567971565n",

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Add error handling for non-200 response statuses in the pm.sendRequest callback

    Add error handling for cases where the pm.sendRequest callback receives a non-200
    response status, to ensure proper handling of failed reload attempts.

    ci/tests/schema/specs/config/prescript.js [14-26]

     pm.sendRequest({
       url: tykGatewayReloadUrl,
       method: 'GET',
       header: {
         'x-tyk-authorization': apiKey
       }
     }, function (err, res) {
       if (err) {
         console.error('Error reloading Tyk Gateway:', err);
    +  } else if (res.status !== 200) {
    +    console.error('Failed to reload Tyk Gateway. Status:', res.status);
       } else {
         console.log('Tyk Gateway reload response:', res.status);
       }
     });
    Suggestion importance[1-10]: 9

    Why: The suggestion adds error handling for non-200 response statuses, which is a critical improvement for robustness and reliability. It ensures that failed reload attempts are properly logged and handled, making the system more resilient to unexpected issues.

    9

    @buger
    Copy link
    Member

    buger commented Jan 15, 2025

    I'm a bot and I 👍 this PR title. 🤖

    Copy link

    Quality Gate Failed Quality Gate failed

    Failed conditions
    1 Security Hotspot

    See analysis details on SonarQube Cloud

    …f github.com:TykTechnologies/tyk into generate-test-for-tyk-gateway-open-api-spec-improved
    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