TykTechnologies / tyk

Tyk Open Source API Gateway written in Go, supporting REST, GraphQL, TCP and gRPC protocols
Other
9.76k stars 1.09k forks source link

[SYSE-394 release-5.3] Fix test code base for Tyk-Analytics PRs #6580

Closed konrad-sol closed 2 months ago

konrad-sol commented 2 months ago

User description

A PR has been created by to address the PRs on tyk-analytics checking out the wrong base (base branch) when working on a pull-request over the tyk-analytics repo.In this case developers would want to checkout the HEAD of the branch instead of the BASE so they can have their new tests in place for the PR.PR being worked out under QA board: https://github.com/TykTechnologies/gromit/pull/346


PR Type

enhancement, configuration changes


Description


Changes walkthrough πŸ“

Relevant files
Enhancement
release.yml
Enhance GitHub Actions workflow with concurrency and conditional logic

.github/workflows/release.yml
  • Added concurrency control for workflow execution.
  • Updated environment variable VARIATION to inverted.
  • Added conditional checks for specific job executions.
  • Enhanced test reporting with S3 upload and summary sharing.
  • +32/-13 
    Configuration changes
    Dockerfile.distroless
    Update base image to Debian Bookworm in distroless Dockerfile

    ci/Dockerfile.distroless
  • Updated base image from debian:bullseye-slim to debian:bookworm-slim.
  • +1/-1     
    Dockerfile.std
    Update base image to Debian Bookworm in standard Dockerfile

    ci/Dockerfile.std
  • Updated base image from debian:bullseye-slim to debian:bookworm-slim.
  • +1/-1     

    πŸ’‘ PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    github-actions[bot] commented 2 months ago

    API Changes

    no api changes detected
    github-actions[bot] commented 2 months ago

    PR Reviewer Guide πŸ”

    ⏱️ Estimated effort to review: 3 πŸ”΅πŸ”΅πŸ”΅βšͺβšͺ
    πŸ§ͺ No relevant tests
    πŸ”’ No security concerns identified
    ⚑ Key issues to review

    Conditional Logic
    The conditional logic for job execution using `if: ${{ matrix.golang_cross == '1.22-bullseye' }}` may not be robust enough if the matrix or dependencies change. Consider a more dynamic approach to handle different versions or configurations. Hardcoded Values
    Hardcoded values in the workflow, such as specific Docker image tags and environment configurations, can lead to maintenance issues. It's better to use variables or parameters that can be easily updated.
    github-actions[bot] commented 2 months ago

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Maintainability
    Replace hardcoded version checks with a variable for better maintainability ___ **Replace the hardcoded '1.22-bullseye' with a variable or a more flexible condition
    to ensure the workflow remains maintainable and adaptable to future changes in the
    Golang version or distribution.** [.github/workflows/release.yml [130]](https://github.com/TykTechnologies/tyk/pull/6580/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R130-R130) ```diff -if: ${{ matrix.golang_cross == '1.22-bullseye' }} +if: ${{ matrix.golang_cross == env.SUPPORTED_GOLANG_VERSION }} ```
    Suggestion importance[1-10]: 8 Why: Replacing hardcoded version checks with a variable improves maintainability and adaptability to future changes, which is a significant enhancement for the workflow.
    8
    Enhancement
    Add error handling to the S3 upload step to enhance workflow robustness ___ **Add error handling for the 'Upload Playwright Test Report to S3' step to manage
    potential failures in the upload process, enhancing the robustness of the workflow.** [.github/workflows/release.yml [346]](https://github.com/TykTechnologies/tyk/pull/6580/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R346-R346) ```diff -run: npm run upload_report_to_s3 +run: | + set -e + npm run upload_report_to_s3 + echo "Upload completed successfully." ```
    Suggestion importance[1-10]: 7 Why: Adding error handling to the S3 upload step increases the robustness of the workflow, which is a valuable improvement, though not critical.
    7
    Use a more descriptive name for the 'VARIATION' environment variable ___ **Consider using a more descriptive name for the 'VARIATION' environment variable to
    clarify its purpose and impact on the workflow.** [.github/workflows/release.yml [27]](https://github.com/TykTechnologies/tyk/pull/6580/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R27-R27) ```diff -VARIATION: inverted +DEPLOYMENT_VARIATION: inverted # More descriptive name for clarity ```
    Suggestion importance[1-10]: 5 Why: Using a more descriptive name for the environment variable can improve code clarity and maintainability, but it is a minor enhancement.
    5
    Possible issue
    Verify and document the change of the 'VARIATION' environment variable ___ **Ensure that the 'VARIATION' environment variable change from 'prod' to 'inverted' is
    intentional and correctly documented, as it could affect the behavior of the
    deployment process.** [.github/workflows/release.yml [27]](https://github.com/TykTechnologies/tyk/pull/6580/files#diff-87db21a973eed4fef5f32b267aa60fcee5cbdf03c67fafdc2a9b553bb0b15f34R27-R27) ```diff -VARIATION: inverted +VARIATION: inverted # Ensure this change is documented and intentional ```
    Suggestion importance[1-10]: 6 Why: Ensuring that changes to environment variables are intentional and documented is important for understanding the deployment process, but it is not a critical issue.
    6
    sonarcloud[bot] commented 2 months ago

    Quality Gate Passed Quality Gate passed

    Issues
    0 New issues
    0 Accepted issues

    Measures
    0 Security Hotspots
    0.0% Coverage on New Code
    0.0% Duplication on New Code

    See analysis details on SonarCloud