Merge pull request #79 from Teamwork/remove-test-mode

Remove the test mode

Author: marcizhu

.github/PULL_REQUEST_TEMPLATE.md

@@ -1,3 +1 @@
 <!-- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change. -->
-
-[Test Task](https://twtest.teamwork.com/#/tasks/20613952) <!-- Dummy test task to make sure the test job runs - replace with real link where necessary. -->

.github/workflows/test.yml

@@ -1,22 +1,15 @@
 name: teamwork
 
 on:
+  push:
+    branches: [master]
   pull_request:
-  pull_request_review:
-    types: [submitted, dismissed]
 
 jobs:
   build:
     runs-on: ubuntu-latest
     name: Ensure this project runs
     steps:
       - uses: actions/checkout@v5
-      - uses: ./
-        with:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-          TEAMWORK_URI: "localhost"
-          TEAMWORK_API_TOKEN: "test_api_token"
-          AUTOMATIC_TAGGING: true
-          WORKFLOW_STAGE_OPENED: "PR Open"
-          WORKFLOW_STAGE_MERGED: "Ready to Test"
-          WORKFLOW_STAGE_CLOSED: "Rejected"
+      - name: Run the tests against a mock Teamwork API
+        run: ./tests/run.sh

CONTRIBUTING.md

@@ -1,34 +1,23 @@
 # Contributing
 
-## Getting started
-
 ## Setup local environment
 
 * [Install Docker](https://docs.docker.com/get-docker/)
-* [Install act tool](https://github.com/nektos/act)
 
-## Usage
+## Running the tests
 
-Simulate a new pull request
+The tests run the action's Docker image against a mock Teamwork API (`tests/mock-teamwork.py`) and check what it sends:
 
 ```
-act pull_request -e tests/events/pull_request_opened.json
+./tests/run.sh
 ```
 
-Simulate a pull request has been approved
+Each directory in `tests/cases` is a test case:
 
-```
-act pull_request_review -e tests/events/pull_request_approved.json
-```
+* `event.json`: the GitHub event the action receives
+* `inputs`: the action's inputs as the `INPUT_*` environment variables GitHub creates for them, plus `GITHUB_EVENT_NAME` and any other environment variables such as `IGNORE_PROJECT_IDS`
+* `expected`: the exit code, errors and requests the mock Teamwork API should end up with
 
-Simulate a pull request has been merged
+`./tests/run.sh <filter>` only runs the cases whose name contains the filter.
 
-```
-act pull_request -e tests/events/pull_request_merged.json
-```
-
-Simulate a pull request has been closed without merging
-
-```
-act pull_request -e tests/events/pull_request_closed.json
-```
+To add a case, create its `event.json` and `inputs`, run `./tests/run.sh --update <case name>` to record its `expected` file, and check that file is right before committing it. The mock's data (which tasks exist, their projects and the workflow stages) is at the top of `tests/mock-teamwork.py`.

README.md

@@ -21,9 +21,7 @@ Create the next environment vars in your repository:
 * `TEAMWORK_URI`: The URL of your installation (e.g.: https://yourcompany.teamwork.com)
 * `TEAMWORK_API_TOKEN`: The API token to authenticate the workflow. Follow [this guide](https://support.teamwork.com/projects/using-teamwork/locating-your-api-key) to find your URL and API key.
 
-**Please Note:** The Teamwork account associated with this API key is the account which these comments will be created under. If this user does not have permission to access the project, this action will be ignored.
-
-`GITHUB_TOKEN` doesn't need to be setup in the repository, this var is always available during the workflows execution.
+**Please Note:** The Teamwork account associated with this API key is the account which these comments will be created under. If this user does not have permission to access a task's project, that task will be skipped and the workflow run will be marked as failed.
 
 ### Installation
 Create a new file `/.github/workflows/teamwork.yml` with the following:
@@ -44,7 +42,6 @@ jobs:
     steps:
       - uses: teamwork/github-sync@master
         with:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           TEAMWORK_URI: ${{ secrets.TEAMWORK_URI }}
           TEAMWORK_API_TOKEN: ${{ secrets.TEAMWORK_API_TOKEN }}
           AUTOMATIC_TAGGING: false
@@ -57,9 +54,9 @@ jobs:
 ```
 
 ## Usage
-When creating a new PR, write in the description of the PR the URL of the task. The action will automatically add a comment in the task. Optionally, you may choose for a lightweight comment that will only include the PR title, URL and the user that performed the action, to enable this option set the `LIGHTWEIGHT_COMMENT` input to `true`.
+When creating a new PR, write in the description of the PR the URL of the task (it must point to the same Teamwork site as `TEAMWORK_URI`, e.g. `https://yourcompany.teamwork.com/app/tasks/123`). The action will automatically add a comment in the task. Optionally, you may choose for a lightweight comment that will only include the PR title, URL and the user that performed the action, to enable this option set the `LIGHTWEIGHT_COMMENT` input to `true`.
 
-Please note, the comment will be created in Teamwork under the account you have attached to this action. If the API key of the user you are using does not have permissions to access certain projects, the comment will not be created.
+Please note, the comment will be created in Teamwork under the account you have attached to this action. If the API key of the user you are using does not have permissions to access certain projects, the comment will not be created and the workflow run will report an error.
 
 ![GitHub pr comment](./.github/assets/github_pr_comment.png)
 

action.yml

@@ -5,8 +5,9 @@ branding:
   color: 'gray-dark'
 inputs:
   GITHUB_TOKEN:
-    description: 'GitHub token'
-    required: true
+    description: 'Deprecated, no longer used.'
+    required: false
+    default: ''
   TEAMWORK_URI:
     description: 'Teamwork URI'
     required: true
@@ -47,15 +48,18 @@ inputs:
 runs:
   using: 'docker'
   image: 'Dockerfile'
+  # Secrets stay out of args, which show up in the runner's process list. The
+  # Teamwork token is read from its INPUT_TEAMWORK_API_TOKEN env var instead,
+  # and the empty slots keep the other args in place.
   args:
-    - ${{ inputs.GITHUB_TOKEN }}
+    - ''
     - ${{ inputs.TEAMWORK_URI }}
-    - ${{ inputs.TEAMWORK_API_TOKEN }}
+    - ''
     - ${{ inputs.AUTOMATIC_TAGGING }}
     - ${{ inputs.WORKFLOW_STAGE_OPENED }}
     - ${{ inputs.WORKFLOW_STAGE_MERGED }}
     - ${{ inputs.WORKFLOW_STAGE_CLOSED }}
     - ${{ inputs.LIGHTWEIGHT_COMMENT }}
     - ${{ inputs.BOARD_COLUMN_OPENED }}
     - ${{ inputs.BOARD_COLUMN_MERGED }}
-    - ${{ inputs.BOARD_COLUMN_CLOSED }}
+    - ${{ inputs.BOARD_COLUMN_CLOSED }}

src/ensure.sh

@@ -1,18 +1,8 @@
 #!/usr/bin/env bash
 
 ensure::env_variable_exist() {
-  if [[ -z "${!1}" ]]; then
-    echoerr "The env variable $1 is required."
-    exit 1
-  fi
-}
-
-ensure::total_args() {
-  local -r received_args=$(( $# - 1 ))
-  local -r expected_args=$1
-
-  if ((received_args != expected_args)); then
-    echoerr "Illegal number of parameters, $expected_args expected but $received_args found"
+  if [[ -z "${!1:-}" ]]; then
+    log::error "The env variable $1 is required."
     exit 1
   fi
 }

src/github.sh

@@ -8,12 +8,9 @@ github::get_action() {
   jq --raw-output .action "$GITHUB_EVENT_PATH"
 }
 
-github::get_pr_number() {
-  jq --raw-output .pull_request.number "$GITHUB_EVENT_PATH"
-}
-
 github::get_pr_body() {
-  jq --raw-output .pull_request.body "$GITHUB_EVENT_PATH" | sed -r 's/^#{1,3}\ /#### /g'
+  jq --raw-output .pull_request.body "$GITHUB_EVENT_PATH" |\
+    awk '/^[[:space:]]*(```|~~~)/ { fenced = !fenced } !fenced { sub(/^#{1,3} /, "#### ") } { print }'
 }
 
 github::get_head_ref() {
@@ -24,10 +21,6 @@ github::get_base_ref() {
   jq --raw-output .pull_request.base.ref "$GITHUB_EVENT_PATH"
 }
 
-github::get_repository_full_name() {
-  jq --raw-output .pull_request.head.repo.full_name "$GITHUB_EVENT_PATH"
-}
-
 github::get_pr_url() {
   jq --raw-output .pull_request.html_url "$GITHUB_EVENT_PATH"
 }
@@ -55,7 +48,3 @@ github::get_review_state() {
 github::get_review_comment() {
   jq --raw-output '.review.body // empty' "$GITHUB_EVENT_PATH"
 }
-
-github::print_all_data() {
-  cat "$GITHUB_EVENT_PATH"
-}

src/main.sh

@@ -12,10 +12,13 @@ main() {
   # Ensure env vars and args exist
   ensure::env_variable_exist "GITHUB_REPOSITORY"
   ensure::env_variable_exist "GITHUB_EVENT_PATH"
+  ensure::env_variable_exist "GITHUB_EVENT_NAME"
 
-  export GITHUB_TOKEN="$1"
+  # $1 used to be GITHUB_TOKEN, which was never needed. The Teamwork token comes
+  # from the env var GitHub creates for the input, so it isn't on the command
+  # line; $3 is the fallback for running the image by hand.
   export TEAMWORK_URI="$2"
-  export TEAMWORK_API_TOKEN="$3"
+  export TEAMWORK_API_TOKEN="${INPUT_TEAMWORK_API_TOKEN:-$3}"
   export AUTOMATIC_TAGGING="$4"
   export WORKFLOW_STAGE_OPENED="$5"
   export WORKFLOW_STAGE_MERGED="$6"
@@ -32,8 +35,6 @@ main() {
   : "${WORKFLOW_STAGE_MERGED:=$BOARD_COLUMN_MERGED}"
   : "${WORKFLOW_STAGE_CLOSED:=$BOARD_COLUMN_CLOSED}"
 
-  env::set_environment
-
   # Check if there is a task link in the PR
   local -r pr_body=$(github::get_pr_body)
   local -r task_ids_str=$(teamwork::get_task_id_from_body "$pr_body" )
@@ -48,37 +49,50 @@ main() {
 
   log::message "Event: $event - Action: $action"
 
+  # Pick the handler before touching the API, so unsupported events make no
+  # requests at all
+  local handler
+  if [ "$event" == "pull_request" ] && [ "$action" == "opened" ]; then
+    handler=teamwork::pull_request_opened
+  elif [ "$event" == "pull_request" ] && [ "$action" == "closed" ]; then
+    handler=teamwork::pull_request_closed
+  elif [ "$event" == "pull_request_review" ] && [ "$action" == "submitted" ]; then
+    handler=teamwork::pull_request_review_submitted
+  elif [ "$event" == "pull_request_review" ] && [ "$action" == "dismissed" ]; then
+    handler=teamwork::pull_request_review_dismissed
+  else
+    log::message "Operation not allowed"
+    exit 0
+  fi
+
+  TEAMWORK_FAILED_REQUESTS=0
+
   local project_id
   IFS=',' read -r -a task_ids <<< "$task_ids_str"
   for task_id in "${task_ids[@]}"; do
     log::message "Task found with the id: $task_id"
 
     export TEAMWORK_TASK_ID=$task_id
-    project_id="$(teamwork::get_project_id_from_task "$task_id")"
+    if ! project_id="$(teamwork::get_project_id_from_task "$task_id")" || [[ ! $project_id =~ ^[0-9]+$ ]]; then
+      teamwork::request_failed "Failed to get the project of task $task_id, check the task exists and the API token's user can access it"
+      continue
+    fi
     export TEAMWORK_PROJECT_ID=$project_id
 
     ignored_project_ids=("${IGNORE_PROJECT_IDS:-}")
     if utils::in_array "$project_id" "${ignored_project_ids[*]}"
     then
         log::message "ignored due to IGNORE_PROJECT_IDS"
-        exit 0
+        continue
     fi
 
-    if [ "$event" == "pull_request" ] && [ "$action" == "opened" ]; then
-      teamwork::pull_request_opened
-    elif [ "$event" == "pull_request" ] && [ "$action" == "closed" ]; then
-      teamwork::pull_request_closed
-    elif [ "$event" == "pull_request_review" ] && [ "$action" == "submitted" ]; then
-      teamwork::pull_request_review_submitted
-    elif [ "$event" == "pull_request_review" ] && [ "$action" == "dismissed" ]; then
-      teamwork::pull_request_review_dismissed
-    elif [ "$ENV" == "test" ]; then # always run pull_request_opened at the very least when in test
-      teamwork::pull_request_opened
-    else
-      log::message "Operation not allowed"
-      exit 0
-    fi
+    "$handler"
   done
 
-  exit $?
+  if ((TEAMWORK_FAILED_REQUESTS > 0)); then
+    log::error "$TEAMWORK_FAILED_REQUESTS Teamwork request(s) failed, see the errors above"
+    exit 1
+  fi
+
+  exit 0
 }

src/misc.sh

@@ -9,14 +9,6 @@ log::message() {
   echo "$@"
 }
 
-env::set_environment() {
-  if [ "$TEAMWORK_URI" == "localhost" ] && [ "$TEAMWORK_API_TOKEN" == "test_api_token" ]; then
-    export ENV="test"
-  else
-    export ENV="prod"
-  fi
-}
-
 utils::in_array() {
   ARRAY=$2
   for e in ${ARRAY[*]}