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

Enable SQL tests #5540

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

Enable SQL tests #5540

wants to merge 3 commits into from

Conversation

ilijabojanovic
Copy link
Member

Description

Enabled PSQL test env for integration tests

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

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

PR Analysis

  • 🎯 Main theme: Enabling SQL tests for integration testing
  • 📝 PR summary: This PR introduces a new GitHub workflow for running API integration tests in a SQL environment. It sets up the necessary environment, checks out the necessary repositories, runs the tests, and provides feedback through various channels.
  • 📌 Type of PR: Tests
  • 🧪 Relevant tests added: Yes
  • 🔒 Security concerns: No

PR Feedback

  • 💡 General suggestions: The PR is well-structured and follows good practices for setting up a CI/CD pipeline. However, it would be beneficial to add some error handling or retries in case some of the steps fail, especially those that depend on external services or repositories. Also, consider adding some comments to the workflow file to explain the purpose of each step, making it easier for others to understand and maintain.

  • 🤖 Code feedback:

    • relevant file: .github/workflows/api-test-sql.yml
      suggestion: Consider adding a step to clean up the environment after the tests have run. This could help save resources and keep the testing environment clean for future runs. [medium]
      relevant line: name: API integration Tests - SQL Environment

    • relevant file: .github/workflows/api-test-sql.yml
      suggestion: It's a good practice to parameterize versions (like Python, Go, Node.js versions) and other reusable values in GitHub Actions. This can make updating versions in the future easier and the workflow file cleaner. [medium]
      relevant line: go-version: [1.19.x]

    • relevant file: .github/workflows/api-test-sql.yml
      suggestion: Consider adding a timeout to the 'start docker compose' step. This can prevent the workflow from hanging indefinitely in case there's an issue with starting the services. [important]
      relevant line: - name: start docker compose

    • relevant file: .github/workflows/api-test-sql.yml
      suggestion: It's a good practice to encrypt sensitive data like secrets. Make sure that the 'ORG_GH_TOKEN' is stored securely and not exposed in logs or other outputs. [important]
      relevant line: TOKEN: '${{ secrets.ORG_GH_TOKEN }}'

How to use

To invoke the PR-Agent, add a comment using one of the following commands:
/review [-i]: Request a review of your Pull Request. For an incremental review, which only considers changes since the last review, include the '-i' option.
/describe: Modify the PR title and description based on the contents of the PR.
/improve [--extended]: Suggest improvements to the code in the PR. Extended mode employs several calls, and provides a more thorough feedback.
/ask <QUESTION>: Pose a question about the PR.
/update_changelog: Update the changelog based on the PR's contents.

To edit any configuration parameter from configuration.toml, add --config_path=new_value
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, use the /config command.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

API Changes

no api changes detected

@Tyk-ITS
Copy link
Contributor

Tyk-ITS commented Sep 5, 2023

API tests result: success
Branch used: refs/pull/5540/merge
Commit:
Triggered by: pull_request (@ilijabojanovic)
Execution page

@Tyk-ITS
Copy link
Contributor

Tyk-ITS commented Sep 5, 2023

API tests result: failure 🚫
Branch used: refs/pull/5540/merge
Commit:
Triggered by: pull_request (@ilijabojanovic)
Execution page

1 similar comment
@Tyk-ITS
Copy link
Contributor

Tyk-ITS commented Sep 5, 2023

API tests result: failure 🚫
Branch used: refs/pull/5540/merge
Commit:
Triggered by: pull_request (@ilijabojanovic)
Execution page

@Tyk-ITS
Copy link
Contributor

Tyk-ITS commented Sep 5, 2023

API tests result: success
Branch used: refs/pull/5540/merge
Commit:
Triggered by: pull_request (@ilijabojanovic)
Execution page

@ilijabojanovic ilijabojanovic enabled auto-merge (squash) September 11, 2023 18:49
@Tyk-ITS
Copy link
Contributor

Tyk-ITS commented Sep 11, 2023

API tests result: success
Branch used: refs/pull/5540/merge
Commit: abb2f5d
Triggered by: pull_request (@ilijabojanovic)
Execution page

@Tyk-ITS
Copy link
Contributor

Tyk-ITS commented Sep 11, 2023

API tests result: success
Branch used: refs/pull/5540/merge
Commit: abb2f5d
Triggered by: pull_request (@ilijabojanovic)
Execution page

@github-actions
Copy link
Contributor

💥 CI tests failed 🙈

git-state

all ok

Please look at the run or in the Checks tab.

@sonarcloud
Copy link

sonarcloud bot commented Sep 13, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@alephnull
Copy link
Contributor

Reiterating my message from Slack here. The approach taken in this PR differs from our approach,

  • we use the images for Tyk components that are shipped to customers
  • we use a versioned image for tyk-automated-tests to reduce the run time
  • we cannot not use a redis dump

@alephnull alephnull removed their request for review December 27, 2023 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants