search

PingCAP-QE / ci

Continue intergration tests
Apache License 2.0
19 stars 96 forks source link

fix: fix br & lightning job name #3016

Closed purelind closed 6 days ago

purelind commented 6 days ago

User description

fix br & lightning job name

PR Type

Bug fix, Enhancement

Description

Changes walkthrough 📝

Relevant files
Bug fix
pull_integration_br_test.groovy
Fix script path for BR integration test job                           
jobs/pingcap/tidb/release-8.2/pull_integration_br_test.groovy - Fixed the script path for the BR integration test job.
+1/-1     
pull_integration_lightning_test.groovy
Fix script path for Lightning integration test job             
jobs/pingcap/tidb/release-8.2/pull_integration_lightning_test.groovy - Fixed the script path for the Lightning integration test job.
+1/-1     
release-8.2-presubmits.yaml
Fix job names and contexts for BR and Lightning integration tests
prow-jobs/pingcap/tidb/release-8.2-presubmits.yaml
  • Fixed job names and contexts for BR and Lightning integration tests.
  • Adjusted job triggers and rerun commands.
    		•  +24/-22 
    Enhancement
    pull_check2.groovy
    Remove artifactVerify parameter from fetchAndExtractArtifact calls
    pipelines/pingcap/tidb/release-8.2/pull_check2.groovy
  • Removed artifactVerify parameter from fetchAndExtractArtifact calls.
    		•  +2/-2     
    pod-pull_check2_debug.yaml
    Remove debug pod configuration file                                           
    pipelines/pingcap/tidb/release-8.2/pod-pull_check2_debug.yaml - Removed the debug pod configuration file.
    		+0/-91   
    pod-pull_check2_v2.yaml
    Remove v2 pod configuration file                                                 
    pipelines/pingcap/tidb/release-8.2/pod-pull_check2_v2.yaml - Removed the v2 pod configuration file.
    		+0/-65   

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

    ti-chi-bot[bot] commented 6 days ago

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

    Based on the pull request, it appears that the changes fix a typo in the job names for pull_integration_br_test.groovy and pull_integration_lightning_test.groovy. Additionally, two files pod-pull_check2_debug.yaml and pod-pull_check2_v2.yaml have been deleted, and the component.fetchAndExtractArtifact function calls in pull_check2.groovy have been modified.

    There are no potential problems with the changes, but the description of the pull request could be more descriptive. It would be helpful if the pull request description provided more information about what was fixed and why it was necessary.

    For fixing suggestions, it is advisable to update the pull request description to include more details about the changes made. Also, the modification to the component.fetchAndExtractArtifact function calls in pull_check2.groovy should be reviewed to ensure that the changes do not impact the functionality of the pipeline.

    codiumai-pr-agent-pro[bot] commented 6 days ago

    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-5] 2
    🧪 Relevant tests No
    🔒 Security concerns No
    ⚡ Key issues to review Possible Bug:
    The removal of the artifactVerify parameter in pull_check2.groovy might introduce risks if artifact verification was a security or integrity measure. Ensure that this change does not compromise the security or correctness of the artifacts being used.
    codiumai-pr-agent-pro[bot] commented 6 days ago

    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 bug
    Reintroduce the trunkBranch and artifactVerify parameters if their removal was unintentional ___ **It appears that the parameters trunkBranch and artifactVerify were removed. If these were
    intentionally removed due to changes in requirements, ignore this suggestion. Otherwise,
    consider adding them back to ensure the functionality remains consistent.** [pipelines/pingcap/tidb/release-8.2/pull_check2.groovy [64-65]](https://github.com/PingCAP-QE/ci/pull/3016/files#diff-36b79bfdc97468ed652103e48f1c087f194942c33313b4f636ac598ef85c28feR64-R65) ```diff -component.fetchAndExtractArtifact(FILE_SERVER_URL, 'tikv', REFS.base_ref, REFS.pulls[0].title, 'centos7/tikv-server.tar.gz', 'bin') -component.fetchAndExtractArtifact(FILE_SERVER_URL, 'pd', REFS.base_ref, REFS.pulls[0].title, 'centos7/pd-server.tar.gz', 'bin') +component.fetchAndExtractArtifact(FILE_SERVER_URL, 'tikv', REFS.base_ref, REFS.pulls[0].title, 'centos7/tikv-server.tar.gz', 'bin', trunkBranch="master", artifactVerify=true) +component.fetchAndExtractArtifact(FILE_SERVER_URL, 'pd', REFS.base_ref, REFS.pulls[0].title, 'centos7/pd-server.tar.gz', 'bin', trunkBranch="master", artifactVerify=true) ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 8 Why: The suggestion correctly identifies the removal of `trunkBranch` and `artifactVerify` parameters, which could impact functionality. Reintroducing these parameters could be crucial if their removal was not intentional.
    		8
    Enhancement
    Adjust the optional field to false for critical integration tests to ensure they are always executed ___ **The optional field for the job pingcap/tidb/release-8.2/pull_br_integration_test and
    pingcap/tidb/release-8.2/pull_lightning_integration_test has been set to true. If these
    tests are critical for integration, consider setting optional to false to ensure they are
    always executed.** [prow-jobs/pingcap/tidb/release-8.2-presubmits.yaml [54]](https://github.com/PingCAP-QE/ci/pull/3016/files#diff-6b7a142781e2dac4873adbff7433d5d0218d143d21fc9c6194c0c3a3b42fd2bbR54-R54) ```diff -optional: true +optional: false ``` - [ ] **Apply this suggestion**
    Suggestion importance[1-10]: 7 Why: The suggestion is valid as it emphasizes the importance of running critical integration tests. However, the decision to make tests optional might be intentional based on the project's current requirements.
    		7
    ti-chi-bot[bot] commented 6 days ago

    [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: - **[jobs/OWNERS](https://github.com/PingCAP-QE/ci/blob/main/jobs/OWNERS)** - **[pipelines/OWNERS](https://github.com/PingCAP-QE/ci/blob/main/pipelines/OWNERS)** - **[prow-jobs/OWNERS](https://github.com/PingCAP-QE/ci/blob/main/prow-jobs/OWNERS)** 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[bot] commented 6 days ago

    [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: - **[jobs/OWNERS](https://github.com/PingCAP-QE/ci/blob/main/jobs/OWNERS)** - **[pipelines/OWNERS](https://github.com/PingCAP-QE/ci/blob/main/pipelines/OWNERS)** - **[prow-jobs/OWNERS](https://github.com/PingCAP-QE/ci/blob/main/prow-jobs/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment