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

Fixed bug with GHA where Update labels do not get applied to Issues with Closed PRs 4839 #4908

Conversation

LRenDO
Copy link
Member

@LRenDO LRenDO commented Jul 2, 2023

Fixes #4839

What changes did you make?

  • Updated conditions for checking for linked pull requests
    • added a condition to check if linked pull request state is open
    • consolidated the conditions that check for an open linked pull request into the variable isOpenLinkedPullRequest
  • Edited and Added log statements to print the state of pull requests linked to the issue
  • Tested issues with various inactivity time frames and linked PR states and verified the intended outcome. Screenshots included are issues after manual workflow runs with test conditions for the closed linked PR bug described. Other test conditions were to ensure no functionality was lost.

Why did you make the changes (we will use this info to test)?

  • When there is an open pull request, we switch focus to checking the PR not the issue, so we can remove the update labels. However, when the issue is still in progress and all of the linked PRs are closed, we need to bring our attention back to the issue by adding update labels. This way we can ensure the issue gets closed if it has been resolved without merging a linked PR or if the issue was not resolved, we can check if the closed PR was a mistake. For that reason, the appropriate update label should be added if the linked PR's are closed.
  • Before the change, the condition only checked if the event was a linked pull request and not the status of the PR. This change ensures that if there isn't an open PR, the issue receives the appropriate update label based on the assignee's most recent activity.
  • Log edits and additions were made to ensure the the log conveys complete and accurate information.
  • Tested issues with various PR states to ensure that the issue was resolved and that prior functionality was retained.

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Visuals before changes are applied

Removes all update labels from issue with closed linked PR requests
image

Visuals after changes are applied

Issues after manual workflow runs:

All Linked Pull Requests Closed 14 Days or More Condition (correctly adds 2 weeks inactive label)
image

Multiple Pull Requests Linked with Last PR Open (correctly removes all update labels, since there is still at least one PR open that we turn our attention to)
image

Multiple Pull Requests Linked with First PR Open (correctly removes all update labels, since there is still at least one PR open that we turn our attention to)
image

Tests Performed

I am including an overview of how I tested as it could be a helpful starting point for creating documentation about testing GHAs in the future. Additionally, the information might help reviewers in catching something I've missed or suggesting improvements for future testing.

The steps used to test were as follows:

  • Copied the project board into a test repo (this action requires a classic project board in the test repo)
  • Added test issues to the repo with the copied project board
  • Moved the all test issue cards to the In Progress (actively working) column of the project board
  • Copied the In Progress (actively working) column link
  • Added a GitHub Action secret called IN_PROGRESS_COLUMN_ID with the copied column id number (column id is the number at the end of the URL)
  • Edited files for manual testing and testing inactivity timeframes (issues were created the same day as testing)
  • In file github/workflows/schedule-fri-0700.yml replaced
    on:
      schedule:
    
    with
    on:
      workflow_dispatch:
      schedule:
    
  • In file github/github-actions/trigger-schedule/add-update-label-weekly/add-label.js cut off times need to be adjusted depending on which inactivity label is being tested
    • To test for Status: Updated label condition (this retains the label so the issue must already be labeled Status: Updated to begin with. It also requires a comment from the assignee.)
      threeDayCutoffTime.setDate(threeDayCutoffTime.getDate() - updatedByDays)
      
      was replaced with
      threeDayCutoffTime.setDate(threeDayCutoffTime.getDate() - updatedByDays + 3)
      
    • To test for To Update ! label condition
      threeDayCutoffTime.setDate(threeDayCutoffTime.getDate() - updatedByDays)
      const sevenDayCutoffTime = new Date()
      sevenDayCutoffTime.setDate(sevenDayCutoffTime.getDate() - commentByDays)
      
      was replaced with
      threeDayCutoffTime.setDate(threeDayCutoffTime.getDate() - updatedByDays + 5)
      const sevenDayCutoffTime = new Date()
      sevenDayCutoffTime.setDate(sevenDayCutoffTime.getDate() - commentByDays + 7)
      
    • To test for 2 weeks in active label condition
      threeDayCutoffTime.setDate(threeDayCutoffTime.getDate() - updatedByDays)
      const sevenDayCutoffTime = new Date()
      sevenDayCutoffTime.setDate(sevenDayCutoffTime.getDate() - commentByDays)
      const fourteenDayCutoffTime = new Date()
      fourteenDayCutoffTime.setDate(fourteenDayCutoffTime.getDate() - inactiveUpdatedByDays)
      
      was replaced with
      threeDayCutoffTime.setDate(threeDayCutoffTime.getDate() - updatedByDays + 7)
      const sevenDayCutoffTime = new Date()
      sevenDayCutoffTime.setDate(sevenDayCutoffTime.getDate() - commentByDays + 9)
      const fourteenDayCutoffTime = new Date()
      fourteenDayCutoffTime.setDate(fourteenDayCutoffTime.getDate() - inactiveUpdatedByDays + 14)
      
  • Pushed the test changes to testing repo with project board setting an upstream feature branch
  • Set default branch to the feature branch
  • Created branches off the feature branch to create enough test pull requests to link to issues
  • Pushed those branches to the test repo
  • Linked PRs to issues, closed PRs without merging, and added labels to issues according to test conditions listed below. (Tested with multiple PRs linked to issues because there have been issues with multiple PRs linked before.)
    • Test conditions:
      • Single open PR linked, inactive issue (removes labels)
      • Open PR linked first with closed PRs, inactive issue (removes labels)
      • Open PR linked last with closed PRs, inactive issue (removes labels)
      • Single closed PR linked, issue inactive inactive 14 days (adds 2 weeks inactive label)
      • Single closed PR linked, issue inactive inactive 7 days (adds To Update ! Label)
      • Closed PRs, issue inactive 14 days (adds 2 weeks inactive label)
      • Closed PRs, issue inactive inactive 7 days (adds To Update ! Label)
      • Assigned issue no PR, inactive 14 days (adds 2 weeks inactive label)
      • Assigned issue no PR, inactive 7 days (adds To Update ! Label)
      • Issue assigned within 7 days (removes all labels)
      • Commented within 3 days (retain Status: Updated label)
  • Ran the workflow manually
  • Checked results
  • Adjusted timeline, issues, and PR state(s) as needed to test any conditions not tested in the first round until all conditions listed were tested.

Testing Resources

@github-actions
Copy link

github-actions bot commented Jul 2, 2023

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b LRenDO-fix-gha-update-labels-with-closed-prs-4839 gh-pages
git pull https://github.com/LRenDO/website.git fix-gha-update-labels-with-closed-prs-4839

@github-actions github-actions bot added Bug Something isn't working role: back end/devOps Tasks for back-end developers Complexity: Large Feature: Board/GitHub Maintenance Project board maintenance that we have to do repeatedly size: 2pt Can be done in 7-12 hours labels Jul 2, 2023
@t-will-gillis t-will-gillis self-requested a review July 3, 2023 15:56
@t-will-gillis
Copy link
Member

eta: 7/4

@roslynwythe roslynwythe self-requested a review July 3, 2023 17:48
@roslynwythe
Copy link
Member

Availability: 3 hrs 7/3
ETA: EOD 7/5

Copy link
Member

@t-will-gillis t-will-gillis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @LRenDO Fantastic job! The branches are correct, the description is linked, the description says what was done and why. I appreciate how your code gives plenty of commentary and logging- this can be very helpful for anyone looking at this months from now. Otherwise, I am able to test this on my own repo. I have issues that should receive a 2 weeks inactive label. I opened PRs and the updating halted as was the intent. I then closed the PRs and the label updating resumed- exactly as it is supposed to.

Great work again and thank you for taking care of this edge case!

@t-will-gillis
Copy link
Member

@LRenDO - I forgot to add something very important: excellent and exemplary documentation of the steps you took, as well as links to testing documents- that is pertinent information that I need to be adding to my issues!

@LRenDO
Copy link
Member Author

LRenDO commented Jul 6, 2023

@t-will-gillis Thanks for the kind and specific feedback! Glad the testing info was a useful addition!

@roslynwythe
Copy link
Member

Hi I'm having some trouble with my test setup. I hope to get some advice tomorrow and complete the review by EOD 7/9

Copy link
Member

@roslynwythe roslynwythe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @LRenDO for a great job on this Large issue. The branches are setup correctly and this PR has excellent description of the problem, solution, and test procedure. I expect to reuse your instructions in a wiki page to help developers and reviewers to test issues related to add-label.js. I tested the code in my repository and found that for Status: Updated, To Update ! and 2 weeks inactive, the behavior of the workflow with a closed PR was identical to the behavior if no PR was linked, and if an open PR is linked, the behavior is unchanged from the current repository.

@roslynwythe roslynwythe merged commit 8af7eca into hackforla:gh-pages Jul 12, 2023
@LRenDO
Copy link
Member Author

LRenDO commented Jul 12, 2023

@roslynwythe Thanks for the review! I'm glad we'll be able to use the testing documentation as a starting point for the wiki for testing this GHA!

ronaldpaek pushed a commit to ronaldpaek/website that referenced this pull request Jul 13, 2023
ronaldpaek pushed a commit to ronaldpaek/website that referenced this pull request Jul 13, 2023
@LRenDO LRenDO deleted the fix-gha-update-labels-with-closed-prs-4839 branch July 16, 2023 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Complexity: Large Feature: Board/GitHub Maintenance Project board maintenance that we have to do repeatedly role: back end/devOps Tasks for back-end developers size: 2pt Can be done in 7-12 hours
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix bug with GHA where Update labels do not get applied to Issues with Closed PRs
3 participants