Merge pull request #22 from product-os/kyle/step-timeout

Remove timeout input and use workflow step timeout-minutes directive

README.md

@@ -12,8 +12,8 @@ where you need manual approval before proceeding with certain actions.
    approval reaction (default: 👍) to approve the workflow.
 3. The action then enters a loop, waiting for a reaction from someone with write
    access to the repository.
-4. If the required reaction is not found, it will continue looping until the job
-   times out.
+4. If the required reaction is not found, it will continue looping until the
+   step times out.
 5. If a denial reaction is received from someone with write access, the action
    will exit with an error.
 
@@ -25,8 +25,6 @@ where you need manual approval before proceeding with certain actions.
   workflow.
 - If the review is approved, the action will log the approver name and continue
   the workflow.
-- If the action times out, it will throw an error and exit the workflow. It can
-  still be re-run manually at this point.
 - Users must have at least `write` access to the repository to have their
   reactions considered as eligible. Read
   [this](https://docs.github.com/en/rest/collaborators/collaborators?apiVersion=2022-11-28#get-repository-permissions-for-a-user)
@@ -52,9 +50,9 @@ To use this action in your workflow, add the following step:
 ```yaml
 - name: Wait for Approval
   uses: product-os/review-commit-action@main
+  timeout-minutes: 60
   with:
-    check-interval: '10'
-    timeout-seconds: 600
+    poll-interval: '10'
     allow-authors: false
 ```
 
@@ -74,10 +72,8 @@ default. Read how to adjust the permissions of the automatic token
 - `github-token`: GitHub token for authentication. The user associated with this
   token is not eligible to review. Uses the actions `GITHUB_TOKEN` secret if
   unset.
-- `check-interval`: Interval in seconds between checks for reactions. Default is
+- `poll-interval`: Interval in seconds between checks for reactions. Default is
   `10`.
-- `timeout-seconds`: Timeout in seconds to wait for eligible reactions. Set to
-  `0` to disable timeout. Overall job timeout takes precedence.
 - `allow-authors`: Allow pull request commit authors to approve or reject the
   workflow. Default is `false`.
 
@@ -115,6 +111,7 @@ jobs:
       - name: Wait for Approval
         uses: product-os/review-commit-action@main
         id: commit-review
+        timeout-minutes: 60
 
       - name: Run after approval
         run: |

__tests__/approval.test.js

@@ -35,7 +35,7 @@ describe('ApprovalProcess', () => {
       commentFooter: 'Test comment footer',
       waitReaction: 'eyes',
       commentHeader: 'Test comment header',
-      checkInterval: 1,
+      pollInterval: 1,
       reviewerPermissions: ['write', 'admin'],
       rejectReaction: '-1',
       approveReaction: '+1',
@@ -147,19 +147,6 @@ describe('ApprovalProcess', () => {
   })
 
   describe('waitForApproval', () => {
-    test('throws an error when timeout is reached', async () => {
-      mockReactionManager.getEligibleReactions.mockResolvedValue([])
-
-      const waitPromise = approvalProcess.waitForApproval(
-        'test-comment-id',
-        0.1,
-        0.3
-      ) // 300ms timeout
-
-      await expect(waitPromise).rejects.toThrow('Approval process timed out')
-      expect(mockReactionManager.getEligibleReactions).toHaveBeenCalledTimes(3)
-    }, 1000) // Set test timeout to 1 second
-
     test('resolves when approved', async () => {
       mockReactionManager.getEligibleReactions.mockResolvedValue([
         { content: '+1', user: { login: 'approver' } }

__tests__/index.test.js

@@ -23,8 +23,7 @@ describe('index.js', () => {
     core.getInput.mockImplementation(name => {
       const inputs = {
         'github-token': 'mock-token',
-        'check-interval': '10',
-        'timeout-seconds': '300'
+        'poll-interval': '10'
       }
       return inputs[name]
     })
@@ -53,8 +52,7 @@ describe('index.js', () => {
       expect.any(ReactionManager),
       expect.objectContaining({
         token: 'mock-token',
-        checkInterval: 10,
-        timeoutSeconds: 300
+        pollInterval: 10
       })
     )
 
@@ -84,8 +82,7 @@ describe('index.js', () => {
     core.getInput.mockImplementation(name => {
       const inputs = {
         'github-token': 'default-token',
-        'check-interval': '',
-        'timeout-seconds': ''
+        'poll-interval': ''
       }
       return inputs[name]
     })
@@ -98,8 +95,7 @@ describe('index.js', () => {
       expect.any(ReactionManager),
       expect.objectContaining({
         token: 'default-token',
-        checkInterval: 10, // default value
-        timeoutSeconds: 0 // default value
+        pollInterval: 10 // default value
       })
     )
   })
@@ -109,8 +105,7 @@ describe('index.js', () => {
     core.getInput.mockImplementation(name => {
       const inputs = {
         'github-token': 'mock-token',
-        'check-interval': 'invalid',
-        'timeout-seconds': 'invalid'
+        'poll-interval': 'invalid'
       }
       return inputs[name]
     })
@@ -123,8 +118,7 @@ describe('index.js', () => {
       expect.any(ReactionManager),
       expect.objectContaining({
         token: 'mock-token',
-        checkInterval: 10, // default value
-        timeoutSeconds: 0 // default value
+        pollInterval: 10 // default value
       })
     )
   })

action.yml

@@ -10,16 +10,10 @@ inputs:
       not eligible to review.'
     required: false
     default: ${{ github.token }}
-  check-interval:
+  poll-interval:
     description: 'Interval in seconds between checks for reactions.'
     required: false
     default: '10'
-  timeout-seconds:
-    description:
-      'Timeout in seconds to wait for eligible reactions. Set to 0 to disable
-      timeout. Overall job timeout takes precedence.'
-    required: false
-    default: '0'
   allow-authors:
     description:
       'Allow pull request commit authors to approve or reject the workflow.'

src/approval.js

@@ -50,7 +50,7 @@ class ApprovalProcess {
     )
 
     try {
-      await this.waitForApproval(comment.id, this.config.checkInterval)
+      await this.waitForApproval(comment.id, this.config.pollInterval)
       await this.reactionManager.setReaction(
         comment.id,
         tokenUser.id,
@@ -67,14 +67,10 @@ class ApprovalProcess {
   }
 
   // Wait for approval by checking reactions on a comment
-  async waitForApproval(commentId, interval = 30, timeout = 0) {
+  async waitForApproval(commentId, interval = 30) {
     const startTime = Date.now()
     Logger.info('Checking for reactions...')
     for (;;) {
-      if (timeout > 0 && (Date.now() - startTime) / 1000 > timeout) {
-        throw new Error('Approval process timed out')
-      }
-
       const reactions =
         await this.reactionManager.getEligibleReactions(commentId)
 

src/index.js

@@ -11,8 +11,7 @@ async function run() {
   try {
     const config = {
       token: core.getInput('github-token'),
-      checkInterval: parseInt(core.getInput('check-interval')) || 10,
-      timeoutSeconds: parseInt(core.getInput('timeout-seconds')) || 0,
+      pollInterval: parseInt(core.getInput('poll-interval')) || 10,
       authorsCanReview: core.getBooleanInput('allow-authors'),
       approveReaction: '+1',
       rejectReaction: '-1',