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

chore(tidb): archive log if test not successful #3028

Merged

Conversation

purelind
Copy link
Collaborator

@purelind purelind commented Jul 12, 2024

User description

archive log if test not successful, eg test timeout


PR Type

enhancement


Description

  • Modified the post-build action in the ghpr_mysql_test.groovy pipeline script to archive logs if the test is unsuccessful instead of only on failure.

Changes walkthrough 📝

Relevant files
Enhancement
ghpr_mysql_test.groovy
Archive logs if test is unsuccessful instead of failure   

pipelines/pingcap/tidb/latest/ghpr_mysql_test.groovy

  • Changed the post-build action from failure to unsuccessful for
    archiving artifacts.
  • +1/-1     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Signed-off-by: purelind <purelind@gmail.com>
    Copy link

    ti-chi-bot bot commented Jul 12, 2024

    I have already done a preliminary review for you, and I hope to help you do a better job.

    Based on the pull request title and description, it seems that the proposed change is to archive logs if the test is not successful.

    Looking at the diff, it appears that the change is modifying a file called ghpr_mysql_test.groovy in the pipelines/pingcap/tidb/latest directory. Specifically, it changes the failure block to unsuccessful block, and archives the artifacts if the test is not successful.

    Overall, the change seems reasonable and should not introduce any issues. However, it might be worthwhile to confirm that the change does not interfere with any other parts of the pipeline that may depend on the failure block.

    As for suggestions, it might be helpful to add some more context to the pull request description. For example, why was this change deemed necessary and how was it tested? Additionally, it might be helpful to include any relevant issue or ticket numbers related to this change.

    @qodo-merge-pro qodo-merge-pro bot added the enhancement New feature or request label Jul 12, 2024
    @ti-chi-bot ti-chi-bot bot added the size/XS label Jul 12, 2024
    Copy link
    Contributor

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified

    Copy link
    Contributor

    PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Replace the unsuccessful block with a specific condition like failure to ensure correct behavior

    Replace the unsuccessful block with a more specific condition such as failure or
    unstable, depending on the desired behavior when archiving artifacts. The
    unsuccessful condition is not standard in Jenkins pipelines and may not work as
    expected.

    pipelines/pingcap/tidb/latest/ghpr_mysql_test.groovy [121-123]

    -unsuccessful {
    +failure {
         archiveArtifacts(artifacts: 'tidb-test/mysql_test/mysql-test.out*', allowEmptyArchive: true)
     }
     
    • Apply this suggestion
    Suggestion importance[1-10]: 10

    Why: The suggestion addresses a critical issue as the unsuccessful condition is not standard in Jenkins pipelines and may lead to unexpected behavior. Replacing it with failure ensures the correct archiving of artifacts.

    10

    Copy link

    ti-chi-bot bot commented Jul 12, 2024

    [APPROVALNOTIFIER] This PR is APPROVED

    Approval requirements bypassed by manually added approval.

    This pull-request has been approved by:

    The full list of commands accepted by this bot can be found here.

    The pull request process is described here

    Needs approval from an approver in each of these files:

    Approvers can indicate their approval by writing /approve in a comment
    Approvers can cancel approval by writing /approve cancel in a comment

    @ti-chi-bot ti-chi-bot bot merged commit 084cf51 into PingCAP-QE:main Jul 12, 2024
    1 of 2 checks passed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    Status: Done
    Development

    Successfully merging this pull request may close these issues.

    1 participant