From 0b5b90cf9327fe7ce0ea1e0c6d873c725ea00c41 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Tue, 29 Aug 2023 15:53:22 -0600 Subject: [PATCH 1/7] first draft of template --- .github/ISSUE_TEMPLATE/update_truth.md | 42 ++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/update_truth.md diff --git a/.github/ISSUE_TEMPLATE/update_truth.md b/.github/ISSUE_TEMPLATE/update_truth.md new file mode 100644 index 0000000000..a3c5672933 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/update_truth.md @@ -0,0 +1,42 @@ +--- +name: Update Truth +about: Review use case differences that are caused by changes in an external repository and update truth dataset if necessary. +title: 'Update Truth: ' +labels: 'alert: NEED ACCOUNT KEY, alert: NEED MORE DEFINITION, alert: NEED CYCLE ASSIGNMENT, type: update truth, priority: blocker, component: CI/CD' +assignees: '' + +--- + +## Describe Expected Changes ## + +*Write a short summary of the differences that will likely be found in the GitHub Actions testing workflow that was triggered by the pull request that warranted this issue* + +## Define the Metadata ## + +### Title ### +- [ ] Add a link to the pull request that warranted this issue to the issue title using format dtcenter/{REPO}#{PR_NUMBER}. + +**Example:** Update Truth: For dtcenter/MET#2655 + +### Assignee ### + +*Assign this issue to the author of the pull request that warranted this issue. Optionally assign anyone else who should review the differences in the output.* + +- [ ] Select **engineer(s)** or **no engineer** required +- [ ] Select **scientist(s)** or **no scientist** required + +### Labels ### +- [ ] Select **component(s)** +- [ ] Select **priority** +- [ ] Select **requestor(s)** + +### Projects and Milestone ### +- [ ] Select **Repository** and/or **Organization** level **Project(s)** or add **alert: NEED CYCLE ASSIGNMENT** label +- [ ] Select **Milestone** as the next official version or **Future Versions** + +## Update Truth Checklist ### +- [ ] Review the GitHub Actions workflow that was triggered by the PR merge + - If no differences were found, note this in a comment. + - If all of the differences are expected, note this in a comment. + - If unexpected differences are found, revert the PR and re-open the issue. +- [ ] Close this issue. From 55d0df33c371c78973bd1742a8216f3e3fbd14e8 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 30 Aug 2023 12:40:45 -0600 Subject: [PATCH 2/7] expanded content of new issue template, rearranged and updated contributor's guide content that is linked in the template --- .github/ISSUE_TEMPLATE/update_truth.md | 18 ++++ docs/Contributors_Guide/add_use_case.rst | 84 ++------------- .../continuous_integration.rst | 100 ++++++++++++++++++ 3 files changed, 126 insertions(+), 76 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/update_truth.md b/.github/ISSUE_TEMPLATE/update_truth.md index a3c5672933..d9f6e69356 100644 --- a/.github/ISSUE_TEMPLATE/update_truth.md +++ b/.github/ISSUE_TEMPLATE/update_truth.md @@ -38,5 +38,23 @@ assignees: '' - [ ] Review the GitHub Actions workflow that was triggered by the PR merge - If no differences were found, note this in a comment. - If all of the differences are expected, note this in a comment. + Include any details of how the review was performed. - If unexpected differences are found, revert the PR and re-open the issue. + Iterate until one of the above conditions apply. +- [ ] Confirm that the truth data can be updated + - Confirm with the METplus wrappers lead engineer (@georgemccabe) or + backup lead (@jprestop) that the truth data can be updated. + - If the engineers are unavailable, follow these instructions to confirm + that the update can be performed: + - Search for other open issues that have the label `type: update truth` + applied by clicking on the label on this issue. Coordinate with the + author of these issues to ensure all diffs are properly reviewed. + - Check if any additional GitHub Actions testing workflows have been + triggered since the workflow that corresponds to this issue was run. + Review the latest run to ensure that there are no diffs that are + unrelated to this issue. +- [ ] Update the truth data. + This may be handled by a METplus wrappers engineer. + See the (instructions to update the truth data)[https://metplus.readthedocs.io/en/develop/Contributors_Guide/continuous_integration.html#update-truth-data-update-truth-data-yml] + for more info. - [ ] Close this issue. diff --git a/docs/Contributors_Guide/add_use_case.rst b/docs/Contributors_Guide/add_use_case.rst index e6445f5944..2deea9dba5 100644 --- a/docs/Contributors_Guide/add_use_case.rst +++ b/docs/Contributors_Guide/add_use_case.rst @@ -656,36 +656,11 @@ Trigger Input Data Ingest If working in a forked METplus repository, the newly added input data will not become available for the tests unless it is triggered from the dtcenter -repository. A METplus developer will need to run the following steps. Please -provide them with the name of the forked repository and the branch that will -be used to create the pull request with the new use case. In this example, -the branch feature_XYZ exists in the *my_fake_user/METplus* repository. First, -clone the *dtcenter/METplus* repository, the run the following:: - - git remote add my_fake_user https://github.com/my_fake_user/METplus - git checkout develop - git checkout -b feature_XYZ - git pull my_fake_user feature_XYZ - git push origin feature_XYZ - git remote remove my_fake_user - -These commands will add a new remote to the forked repository, create a branch -off of the develop branch with the same name as the branch on the fork, pull -in the changes from the forked branch, then push the new branch up to -*dtcenter/METplus* on GitHub. Finally, the remote is removed to avoid clutter. - -Once these steps have been completed, go to *dtcenter/METplus* on GitHub -in a web browser and navigate to the -`Actions tab `_. -Click on the job named -"Docker Setup - Update Data Volumes" then click on "Update Data Volumes" and -verify that the new data tarfile was found on the DTC web server and the new -Docker data volume was created successfully. See -:ref:`verify-new-input-data-was-found`. If the input data was ingested -properly, then delete the feature branch from *dtcenter/METplus*. -This will avoid -confusion if this branch diverges from the branch on the forked repository that -will be used in the final pull request. +repository. A METplus developer will need to run the +:ref:`cg-ci-update-input-test-data` GitHub Actions workflow to trigger it. +Please provide them with the name of the branch that will +be used to create the pull request with the new use case. + .. _add_use_case_to_test_suite: @@ -1143,52 +1118,9 @@ Update the Truth Data The addition of a new use case results in new output data. When this happens, the reference branch needs to be updated so that future pull requests will compare their results to a "truth" data set that contains the new files. -Create a pull request with "develop" as the source branch and "develop-ref" as -the destination branch. This is done so that the pull request number -responsible for the changes in the truth data can be referenced to easily -track where differences occurred. - -A GitHub Action workflow is available to handle this step. - -* Ensure that the develop data directory has been updated to include all of the - new input data. - Check with the reviewers of recent pull requests that add a new use case to - confirm that the steps under :ref:`update-the-develop-data-directory` have - been completed. If this step has not been completed, then the new use case(s) - will fail and the new output data will not be added to the truth data set. -* Navigate to https://github.com/dtcenter/METplus/actions/workflows/update_truth.yml - or from the METplus GitHub page, click on the Actions tab, - then click on "Update Truth Data" under menu on the left. -* Click on the "Run workflow" button on the right. -* Click on the Branch pull down and select "develop" unless you are updating - the truth data for a bugfix on a main_vX.Y branch. -* Enter the pull request numbers that warranted the update. - Include the '#' symbol before the number to create a link to the PR. - PRs from a repository other than METplus should include - the repository name before '#' symbol. -* Enter a brief summary of the changes. - Developers can navigate to the PRs for more information. - -.. figure:: figure/update_truth_data.png - -* Click the "Run workflow" button. -* A new workflow run should appear at the top of the list and complete quickly. -* Click on the "Pull Requests" tab. - A new pull request should have been created with the information that - was entered. Click on the new pull request. -* Verify that the information in this pull request is correct. - If the "develop" branch was selected in the "Run workflow" menu, - then the pull request should show **develop-ref <- develop**. -* Add the appropriate project and milestone values on the right hand side. -* Scroll to the bottom of the pull request and click "Squash and merge." -* Click "Confirm squash and merge." It is not necessary to wait for the - automation checks to complete for this step. -* Monitor the Testing automation run for the develop-ref branch and ensure that - all of the use cases run successfully and the final step named - "Create Output Docker Data Volumes" completed successfully. -* If any use cases fail, check that the input data has been updated following - the instructions under :ref:`update-the-develop-data-directory` and rerun - all of the jobs of the -ref workflow. + +Follow the instructions for using the :ref:`cg-ci-update-truth-data` GitHub +Actions workflow to perform this step. Clean Up DTC Web Server diff --git a/docs/Contributors_Guide/continuous_integration.rst b/docs/Contributors_Guide/continuous_integration.rst index 0fbe5fff72..6911811a07 100644 --- a/docs/Contributors_Guide/continuous_integration.rst +++ b/docs/Contributors_Guide/continuous_integration.rst @@ -76,6 +76,106 @@ at the bottom of the workflow summary page when the workflow has completed. .. figure:: figure/ci-doc-artifacts.png +.. _cg-ci-update-truth-data: + +Update Truth Data (update_truth.yml) +------------------------------------ + +The METplus use case test truth data includes output from use cases that is +used to compare with new use case test results to flag any differences. +Differences can occur due to changes to the METplus wrappers +source code/configuration files or changes to any of its dependent +METplus components such as MET, METplotpy, METcalcpy, and METdataio. +Differences can also occur when a new use case is added, as the new use case +creates output that does not yet exist in the truth dataset. + +Once all differences are confirmed to be expected, +the reference branch, e.g. develop-ref, needs to be updated. This triggers a +:ref:`cg-ci-testing-workflow` that runs all of the use cases, then creates +Docker images that contain the new truth data and are pushed to DockerHub. +This is done so that future pull requests will +compare their results to the updated truth dataset. + +This process involves creating a pull request with "develop" as the source +branch and "develop-ref" as the destination branch. +This is done so that the pull request responsible for the changes in the +truth data can be referenced to easily track where differences occurred. + +The **Update Truth Data** workflow is available to handle this step. + +* Ensure that the develop data directory has been updated to include all of the + new input data. + Check with the reviewers of recent pull requests that add a new use case to + confirm that the steps under :ref:`update-the-develop-data-directory` have + been completed. If this step has not been completed, then the new use case(s) + will fail and the new output data will not be added to the truth data set. +* Navigate to https://github.com/dtcenter/METplus/actions/workflows/update_truth.yml + or from the METplus GitHub page, click on the Actions tab, + then click on "Update Truth Data" under menu on the left. +* Click on the "Run workflow" button on the right. +* Click on the Branch pull down and select "develop" unless you are updating + the truth data for a bugfix on a main_vX.Y branch. +* Enter the pull request numbers that warranted the update. + Include the '#' symbol before the number to create a link to the PR. + PRs from a repository other than METplus should include + the repository name before '#' symbol. +* Enter a brief summary of the changes. + Developers can navigate to the PRs for more information. + +.. figure:: figure/update_truth_data.png + +* Click the "Run workflow" button. +* A new workflow run should appear at the top of the list and complete quickly. +* Click on the "Pull Requests" tab. + A new pull request should have been created with the information that + was entered. Click on the new pull request. +* Verify that the information in this pull request is correct. + If the "develop" branch was selected in the "Run workflow" menu, + then the pull request should show **develop-ref <- develop**. +* Add the appropriate project and milestone values on the right hand side. +* If a GitHub issue exists to track the review of the differences, click on + the gear icon next to *Development* on the right side menu and add the issue. +* Scroll to the bottom of the pull request and click "Squash and merge." +* Click "Confirm squash and merge." It is not necessary to wait for the + automation checks to complete for this step. +* Monitor the Testing automation run for the develop-ref branch and ensure that + all of the use cases run successfully and the final step named + "Create Output Docker Data Volumes" completed successfully. +* If any use cases fail, check that the input data has been updated following + the instructions under :ref:`update-the-develop-data-directory` and rerun + all of the jobs of the -ref workflow. + +.. _cg-ci-update-input-test-data: + +Update Input Test Data (update_input_data.yml) +---------------------------------------------- + +New/updated input data for a METplus use case is read from the +DTC web server as described in the :ref:`use_case_input_data` section of the +**Adding Use Cases** chapter of the METplus Contributor's Guide. +This automatically happens as part of the :ref:`cg-ci-testing-workflow` when +a push event occurs on a dtcenter/METplus branch. +This step can be forced by using the **Update Input Test Data** workflow. + +This is workflow is typically used when a new use case is being provided by +an external contributor and their pull request is coming from a forked +repository. +Only dtcenter/METplus workflows have permission to update the input test data. + +To force the ingest of this input data, navigate to +https://github.com/dtcenter/METplus/actions/workflows/update_input_data.yml . +Click on the **Run workflow** pull-down, type the name of the branch that +matches the directory that contains the new data on the DTC web server, +and click the **Run workflow** button. + +The value in the branch pull-down under the text that says +**Use workflow from** is ignored if there is a value typed for the branch name. +If the branch name exists in the dtcenter/METplus repository, leave the +branch name text box blank and select the branch name from the pull-down menu. + +Verify that the workflow ran successfully and properly obtained the new data +by reviewing the log output from the workflow run. + Release Published (release_published.yml) - DEPRECATED ------------------------------------------------------ From f2e3c53ab3a6ca70b6f2bdc3e8141a882d3c235d Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 30 Aug 2023 16:03:12 -0600 Subject: [PATCH 3/7] Update docs/Contributors_Guide/continuous_integration.rst Co-authored-by: John Halley Gotway --- docs/Contributors_Guide/continuous_integration.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Contributors_Guide/continuous_integration.rst b/docs/Contributors_Guide/continuous_integration.rst index 6911811a07..df486ee655 100644 --- a/docs/Contributors_Guide/continuous_integration.rst +++ b/docs/Contributors_Guide/continuous_integration.rst @@ -92,7 +92,7 @@ creates output that does not yet exist in the truth dataset. Once all differences are confirmed to be expected, the reference branch, e.g. develop-ref, needs to be updated. This triggers a :ref:`cg-ci-testing-workflow` that runs all of the use cases, then creates -Docker images that contain the new truth data and are pushed to DockerHub. +Docker images with the new truth data, and pushes them to DockerHub. This is done so that future pull requests will compare their results to the updated truth dataset. From 91735546d737ba902dc7dd9580143b6d41fbf7a6 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 30 Aug 2023 16:04:07 -0600 Subject: [PATCH 4/7] Update docs/Contributors_Guide/continuous_integration.rst Co-authored-by: John Halley Gotway --- docs/Contributors_Guide/continuous_integration.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Contributors_Guide/continuous_integration.rst b/docs/Contributors_Guide/continuous_integration.rst index df486ee655..b4283f9a97 100644 --- a/docs/Contributors_Guide/continuous_integration.rst +++ b/docs/Contributors_Guide/continuous_integration.rst @@ -91,7 +91,7 @@ creates output that does not yet exist in the truth dataset. Once all differences are confirmed to be expected, the reference branch, e.g. develop-ref, needs to be updated. This triggers a -:ref:`cg-ci-testing-workflow` that runs all of the use cases, then creates +:ref:`cg-ci-testing-workflow` that runs all of the use cases, creates Docker images with the new truth data, and pushes them to DockerHub. This is done so that future pull requests will compare their results to the updated truth dataset. From 69a2472caeac35d82bb5963b98e4e2a6f25024db Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 30 Aug 2023 16:05:36 -0600 Subject: [PATCH 5/7] Update .github/ISSUE_TEMPLATE/update_truth.md Co-authored-by: John Halley Gotway --- .github/ISSUE_TEMPLATE/update_truth.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/ISSUE_TEMPLATE/update_truth.md b/.github/ISSUE_TEMPLATE/update_truth.md index d9f6e69356..5774b1beca 100644 --- a/.github/ISSUE_TEMPLATE/update_truth.md +++ b/.github/ISSUE_TEMPLATE/update_truth.md @@ -2,7 +2,7 @@ name: Update Truth about: Review use case differences that are caused by changes in an external repository and update truth dataset if necessary. title: 'Update Truth: ' -labels: 'alert: NEED ACCOUNT KEY, alert: NEED MORE DEFINITION, alert: NEED CYCLE ASSIGNMENT, type: update truth, priority: blocker, component: CI/CD' +labels: 'alert: NEED ACCOUNT KEY, alert: NEED MORE DEFINITION, alert: NEED CYCLE ASSIGNMENT, type: update truth, priority: blocker, component: CI/CD, requestor: METplus Team' assignees: '' --- From d22626da5bd4d618f12d694772d64c0eaa5ebf57 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 30 Aug 2023 16:06:47 -0600 Subject: [PATCH 6/7] remove labels from checklist because they are already set automatically --- .github/ISSUE_TEMPLATE/update_truth.md | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/update_truth.md b/.github/ISSUE_TEMPLATE/update_truth.md index 5774b1beca..2a036dbc74 100644 --- a/.github/ISSUE_TEMPLATE/update_truth.md +++ b/.github/ISSUE_TEMPLATE/update_truth.md @@ -25,11 +25,6 @@ assignees: '' - [ ] Select **engineer(s)** or **no engineer** required - [ ] Select **scientist(s)** or **no scientist** required -### Labels ### -- [ ] Select **component(s)** -- [ ] Select **priority** -- [ ] Select **requestor(s)** - ### Projects and Milestone ### - [ ] Select **Repository** and/or **Organization** level **Project(s)** or add **alert: NEED CYCLE ASSIGNMENT** label - [ ] Select **Milestone** as the next official version or **Future Versions** From ef3d1845c5781928c3c4b9bdae6dc6bc97f67988 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 30 Aug 2023 16:16:12 -0600 Subject: [PATCH 7/7] adjusted the checklist for confirming diffs --- .github/ISSUE_TEMPLATE/update_truth.md | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/.github/ISSUE_TEMPLATE/update_truth.md b/.github/ISSUE_TEMPLATE/update_truth.md index 2a036dbc74..0b18a6cd88 100644 --- a/.github/ISSUE_TEMPLATE/update_truth.md +++ b/.github/ISSUE_TEMPLATE/update_truth.md @@ -34,13 +34,10 @@ assignees: '' - If no differences were found, note this in a comment. - If all of the differences are expected, note this in a comment. Include any details of how the review was performed. - - If unexpected differences are found, revert the PR and re-open the issue. - Iterate until one of the above conditions apply. -- [ ] Confirm that the truth data can be updated - - Confirm with the METplus wrappers lead engineer (@georgemccabe) or - backup lead (@jprestop) that the truth data can be updated. - - If the engineers are unavailable, follow these instructions to confirm - that the update can be performed: + - If unexpected differences are found, the following instructions can + help uncover potential explanations. If none of these apply and the + source of the differences cannot be determined, contact the + METplus wrappers lead engineer (@georgemccabe) for assistance. - Search for other open issues that have the label `type: update truth` applied by clicking on the label on this issue. Coordinate with the author of these issues to ensure all diffs are properly reviewed. @@ -48,8 +45,16 @@ assignees: '' triggered since the workflow that corresponds to this issue was run. Review the latest run to ensure that there are no diffs that are unrelated to this issue. + - If the incorrect differences are caused by the changes from the + issue that warranted this issue, consider reverting the PR and + re-opening the issue. + - Iterate until one of the above conditions apply. +- [ ] Approve the update of the truth data + - Contact the METplus wrappers lead engineer (@georgemccabe) or + backup lead (@jprestop) to let them know that the truth data can + be updated. - [ ] Update the truth data. - This may be handled by a METplus wrappers engineer. + This should be handled by a METplus wrappers engineer. See the (instructions to update the truth data)[https://metplus.readthedocs.io/en/develop/Contributors_Guide/continuous_integration.html#update-truth-data-update-truth-data-yml] for more info. - [ ] Close this issue.