Add actions and workflows for QIT tests #81

tomalec · 2023-09-27T19:58:20Z

Changes proposed in this Pull Request:

This PR adds workflows and actions to be used here and in extensions' repos.

run-qit-annotate action is a basic building block. It takes test type, extension name, and optionally extension-file, to run a qit test, wait for results, and propagate them to annotations and output. It also takes a few optional params for customization:
- options - so a user can provide additional options to qit command, like --optional_features=hpos, or --woocommerce_version (currently broken Cannot provide WP & WC versions as suggested in the docs qit-cli#81)
- wait to wait for the results, otherwise it just fires the test run, and returns its id for further checks
- ignore-fail a flag to consider a failed test "ok" thing, for example, to proceed with other test types
run-qit-extension is another action to run all test types for a given extension. It takes qit-partner-* secrets, set of run-qit-annotate inputs (extension, wait, ignore-fail, options) and test-* flag for each type to run. It also takes version input:
- latest will test the latest publicly released version,
- dev will make the action look up for an {extension}.zip file in the root of the repository under the gha-dev-build tag,
- local will look up for an {extension}.zip artifact in the current workflow.
.github/workflows/run-qit-all.yml local manual workflow that uses the above to be run on demand to test all the extensions at once. Currently, it's configured to test Automata extensions.

To make those workflows run, three secrets are required to be set up:

BOT_GH_TOKEN
QIT_PARTNER_USER
QIT_PARTNER_SECRET

Screenshots:

Detailed test instructions:

Manual workflow

Fork the repo
Setup secrets. Links to secrets are at Pe98dy-Ir-p2
Merge this branch to the main one /trunk
Run the manual workflow at https://github.com/woocommerce/grow/actions/workflows/run-qit-all.yml

Reusable action

See woocommerce/google-listings-and-ads#2114

Use a repo that builds a QIT-compatible extension
make sure you have another workflow that builds it and publishes the artifact if you want to test local version. You can test latest and dev without this step.
Use this workflow

jobs:
  build:
    name: Build extension
    uses: ./.github/workflows/build.yml
    secrets: inherit
  qit-tests:
    name: Run QIT Tests
    runs-on: ubuntu-20.04
    needs: build
    steps:
      - name: Download artifact
        uses: actions/download-artifact@v3
        with:
          name: automatewoo.zip
      - name: Delegate QIT Tests
        uses: woocommerce/grow/run-qit-extension@add/qit-workflows-test-build
        with:
          qit-partner-user: ${{ secrets.QIT_PARTNER_USER }}
          qit-partner-secret: ${{ secrets.QIT_PARTNER_SECRET }}
          version: local
          wait: true
          extension: 'automatewoo'
          test-activation: true
          test-security: true
          ignore-fail: true
          options: '--optional_features=hpos'

Additional details:

Manual workflow can be tested only once merged to the main branch. So you can try it on your local fork.
~~Due to the above, for show-case purposes, I run the workflow on the versions matrix for each push to this branch. https://github.com/woocommerce/grow/actions/runs/6331342166~~
We publish here a reusable workflow. I was thinking of pushing it to github-actions folder. So, we will have it under common release flow and location, with more convenient version tags. But I'm not sure, as this is "workflow" not "action". Maybe we can store it in a location like .github/workflows/reusable or packages/workflows and preserve that folder in the github-actions release? (edit: as per the discussion below, we decided to go with a another composite action)
⚠️ before merging, we need to update references from woocommerce/grow/run-qit-*@add/qit-workflows-test-build to woocommerce/grow/run-qit-annotate@actions-v1

Changelog entry

Add QIT workflows and actions.

remove `on push` used for testing.

eason9487

Thanks for the work.

The current way of transferring a local extension zip might not be easy to handle in the future in terms of version compatibility. Wonder if it would be better to leave the processing to the action user side.

Another concern is about the security reports. Since this repo is public, the security test reports for the private repos such as AW will also reveal the report URLs after running .github/workflows/run-qit-all.yml.

For example, it reveals the "****" POST parameter is not sanitized. (I replaced the actual info with **** via DevTool)

I'd like to clarify whether this is appropriate?

eason9487 · 2023-10-02T10:30:19Z

.github/workflows/run-qit-extension.yml

+            - name: Download artifact
+              if: ${{ inputs.version == 'local' }}
+              uses: actions/download-artifact@v3
+              with:
+                name: ${{ env.dev_build }}


If I guess it right, this step is meant to be used in a way like this line?

A concern is the workflow files in .github/workflows directory of this repo won't be included in the actions-v1 releases. As a result, it would become difficult to ensure version compatibility between them.

To run QIT tests with a local build, I would suggest leaving the processing of preparing a local extension zip file on the action user side. For example, GLA could either build a zip file within its workflow or run the actions/download-artifact step on its own, and this action only needs the local file path as input.

Yes, this is the way to use it.

A concern is the workflow files in .github/workflows directory of this repo won't be included in the actions-v1 releases. As a result, it would become difficult to ensure version compatibility between them.

I completely agree with that concern. That's why in the OP I mentioned:

I was thinking of pushing it to github-actions folder. So, we will have it under common release flow and location, with more convenient version tags. But I'm not sure, as this is "workflow" not "action". Maybe we can store it in a location like .github/workflows/reusable or packages/workflows and preserve that folder in the github-actions release?

I would appreciate your opinion here. I'd say having separate release flows for workflows and separate for actions is too much. I would choose one of those approaches:

Consider our [github] actions tag actually mean "github actions & workflows", then

consider workflows and atomic actions completely equal, and make consumers distinguish what is what:

jobs: build: runs-on: ubuntu-latest steps: - name: Reuse action uses: woocommerce/grow/prepare-php@actions-v1 # Action - should be used in `jobs.*.steps.uses` runQIT: name: Reuse workflow uses: woocommerce/grow/run-qit-extension.yml@actions-v1 # Workflow - should be used in `jobs.*.uses` needs: build

put workflows and actions into distinct folders (so maintainers and consumers can easily distinguish what is what)

jobs: build: runs-on: ubuntu-latest steps: - name: Reuse action uses: woocommerce/grow/action/prepare-php@actions-v1 runQIT: name: Reuse workflow uses: woocommerce/grow/workflow/run-qit-extension.yml@actions-v1 needs: build

To run QIT tests with a local build, I would suggest leaving the processing of preparing a local extension zip file on the action user side. For example, GLA could either build a zip file within its workflow or run the actions/download-artifact step on its own, and this action only needs the local file path as input.

The problem here is that. This is a workflow (jobs.*.uses), not an atomic action (jobs.*.steps.uses). This workflow runs as a separate job, from any other workflow or action. So the only way to share a file (I'm aware of) between this workflow and another (GLA's, or other extension's) workflow is to use artifacts. Then this workflow needs to download them itself. Otherwise, how would GLA's job push a file to this job?

The version compatibility concern in #81 (comment) is the same as this thread.

The current way of transferring a local extension zip might not be easy to handle in the future in terms of version compatibility. Wonder if it would be better to leave the processing to the action user side.

Could you elaborate on that?

Sorry, I omitted a large portion of explanation. The idea is to transform the run-qit-extension.yml workflow into a composite action and it involves a degree of change that is not quite easily stated in words. I created a demo in the repo and one in GLA to illustrate how it works.

Changes in this repo

Based on the run-qit-extension workflow, create a composite run-qit-extension action.

Remove the "Download artifact" step.

Add qit-partner-user and qit-partner-secret inputs to forward the original secret variables.

Remove all timeout-minutes as they are not supported in the composite action. Probably it would be better to let the action user specify the timeout.

I didn't transform the outputs.statuses as it would need more time or the composite action might not support the same syntax.

grow/.github/workflows/run-qit-extension.yml

Lines 52 to 55 in a8f23ba

outputs:

statuses:

description: "Statuses of all tests. Array of integers."

value: ${{ jobs.qit-tests.outputs.statuses }}

grow/.github/workflows/run-qit-extension.yml

Lines 65 to 66 in a8f23ba

outputs:

statuses: ${{ toJSON( steps.*.outputs.status ) }}

Changes in GLA

Add the "Download artifact" step.

Use the run-qit-extension action instead.

The test run: https://github.com/woocommerce/google-listings-and-ads/actions/runs/6391261008

I didn't continue to make further changes. If adopt this method, the .github/workflows/run-qit-all.yml workflow should be able to use run-qit-extension action as well, and the .github/workflows/run-qit-extension.yml workflow would be no longer in needed.

In this way, the action user sides won't need to use this repo's workflow directly, and this repo doesn't need to add them to any release and versioning flow.

Another advantage is that the action user side wouldn't have to upload artifacts if their action builds a local zip file and uses this action in the same job. For example:

# ... omitted jobs: qit-tests: name: Run QIT Tests runs-on: ubuntu-20.04 steps: - name: Checkout repository uses: actions/checkout@v3 # ... omitted - name: Build production bundle run: | npm run build - name: Delegate QIT Tests uses: woocommerce/grow/run-qit-extension@actions-v1 with: qit-partner-user: ${{ secrets.QIT_PARTNER_USER }} qit-partner-secret: ${{ secrets.QIT_PARTNER_SECRET }} version: 'local' extension: 'google-listings-and-ads' test-security: 'true'

Thank you for the detailed explanations and the new branch!
I can see it works, and I appreciate how it addresses the following:

this repo doesn't need to add them [workflows] to any release and versioning flow.

However, I'm not sure if I see all the benefits of

the action user sides won't need to use this repo's workflow directly

I get that accessing the workflow directly is not great, and I'm happy to improve it. But I think we can to it also otherwise

Using action or workflow from code perspective is not much longer

The biggest difference I see is granularity. Having qit as an atomic action, users could compose it within their workflows.
But I think it also has the downsides:

the log is less collapsible and divided.:
as a workflow:

as an action

I was actually thinking of unifying the workflow across our repos so we will not have to implement a workflow and compose actions in every repo but just use the shared entire workflow.

Especially if it comes to the build, I think it is even desirable to have it as a separate workflow that uploads an artifact. So, other workflows and actions running for the same commit could reuse that work, reducing the overall cost and time.
See woocommerce/google-listings-and-ads#2114 (comment)

I was actually thinking of unifying the workflow across our repos so we will not have to implement a workflow and compose actions in every repo but just use the shared entire workflow.

From the perspective of this repo, I would suppose its granularity prefers a custom action over a reusable workflow for flexibility in composition and configuration and fewer limitations.

If an automation requirement is highly repetitive or identical across several repos, then a reusable workflow, which might still composite some custom actions within this repo, would be appropriate.

Especially if it comes to the build, I think it is even desirable to have it as a separate workflow that uploads an artifact. So, other workflows and actions running for the same commit could reuse that work, reducing the overall cost and time. [...]

From a practicable point of view, I find it rather difficult to imagine whether it would be easier to do so.

Take GLA as an example, given it has a reusable workflow for building a zip bundle, and each automation of "Bundle watch", "Publish dev build", "E2E test", and "QIT" need a build file as input.

Unless all these automations are in the same workflow, each automation will be grouped into standalone workflows with the same execution conditions, and then build a zip on its own. Because artifact access through actions/upload-artifact and actions/download-artifact is limited within the same workflow run. In this case, the same commit doesn't use the "same" work result (zip bundle), only the extracted build workflow is reused.

However, having all of them in the same workflow run seems to make that workflow configuration very complex.

My main concern is still how the release versions of these reusable workflows in the .github/workflows directory will proceed if go with the current solution.

The unintuitive understanding of the artifact upload/download split between the use-end repo and this repo is also a concern, but if this complexity trade-off is necessary, then it won't be a blocker.

I'd say having separate release flows for workflows and separate for actions is too much.

Agree with this.

I would choose one of those approaches: [...]

GitHub docs say the syntax of jobs.<job_id>.uses is {owner}/{repo}/.github/workflows/{filename}@{ref} and doesn't clarify if the .github/workflows/ part can be changed.

Either yes or no, I lean toward leaving reusable workflows under the original directory and keeping custom actions the same as it's:

jobs: build: runs-on: ubuntu-latest steps: - name: Reuse action uses: woocommerce/grow/prepare-php@actions-v1 runQIT: name: Reuse workflow uses: woocommerce/grow/.github/workflows/run-qit-extension.yml@actions-v1 needs: build

The main reason is that each of them conforms to the syntax described in GitHub docs respectively.

jobs.<job_id>.steps[*].uses

jobs.<job_id>.uses

I merged your proposed changes and used action as you suggested.
We can get back to shared workflows if needed in some following PRs, exploring possibilities and flow optimizations there.

packages/github-actions/actions/run-qit-annotate/README.md

Co-authored-by: Eason <eason.su.tw@gmail.com>

tomalec · 2023-10-02T19:24:04Z

Thank you for the deep review :)

The current way of transferring a local extension zip might not be easy to handle in the future in terms of version compatibility. Wonder if it would be better to leave the processing to the action user side.

Could you elaborate on that? What challenges do you see with that approach in regard to version compatibility? BTW, by "version compatibility" do you mean "testing extension's compatibility with WC/WP version", or something else?
How would you make it so that the user would push the extension zip to another workflow?

tomalec · 2023-10-02T19:51:16Z

Another concern is about the security reports. Since this repo is public, the security test reports for the private repos such as AW will also reveal the report URLs after running .github/workflows/run-qit-all.yml.
...
I'd like to clarify whether this is appropriate?

Thanks for catching that! I confirmed with Masamune Team, that test results should be considered private to the direction of the extension vendor. So for 3PD QIT access, a vendor can see theirs and only their extension results. And it's up to the vendor what they do with the results.

I posted a comment on the project thread so we could discuss or approach and policy peeuvX-QJ-p2#comment-1215

I'll look for ways to hide those results

eason9487 · 2023-10-03T10:41:47Z

Just noticed the indentation of some .yml files is mixed with 2-space and 4-space units. It would be better to make them consistent.

packages/github-actions/actions/run-qit-annotate/action.yml

tomalec · 2023-10-26T19:54:40Z

Speaking of reusable entire workflows, I think we could benefit from having them for other cases like .github/workflows/js-css-linting.yml

Use changes added in woocommerce/qit-cli#109 Add `ignore_fail` parameter, to simplify continue-on-error behavior. Drop the report file, use annotations only. Hide result URLs, expose `test_run_id` for further checks.

Addresses #81 (review)

Update docs.

tomalec · 2024-01-31T02:51:09Z

Speaking of hiding fragile data. Now, we echo only parsed json output, that contains {test_run_id, status, test_summary}, like

{
  "test_run_id": 4554116,
  "status": "failed",
  "test_summary": "Errors: 0, File Errors: 86"
}

I believe it's the right balance between the developer's convenience and exposing vulnerabilities.. The annotation also contains a ready CLI command to check for more, like:

So, we no longer expose the results URL with a private token, but only test_run_id. IMHO, it should not be considered private, as it's just a consecutive number, so anybody could guess that. Then, it's up to QIT CLI to guard against unauthorized qit get ...

tomalec · 2024-01-31T02:52:59Z

@eason9487 Thank you once again for the detailed review and great insights. Also, thank you for the "GitHub Actions - Create Test Build" - it made development and testing much easier.

I addressed all your comments, and the PR is ready for another round of review.

eason9487

Thanks for the great work! LGTM.

Left two minor comments and they are not blockers.

eason9487 · 2024-02-01T02:38:54Z

packages/github-actions/README.md

@@ -20,6 +20,7 @@ Custom GitHub actions that help to composite GitHub workflows across the repos m
 - [`prepare-node`](actions/prepare-node) - Set up Node.js with a specific version, load npm cache, install Node dependencies
 - [`prepare-php`](actions/prepare-php) - Set up PHP with a specific version and tools, load Composer cache, install Composer dependencies
 - [`publish-extension-dev-build`](actions/publish-extension-dev-build) - Publish extension development build
+- [`run-qit-annotate`](actions/run-qit-annotate) - Runs QIT test and annotates the results


The run-qit-extension action could be listed here as well.

eason9487 · 2024-02-01T02:42:55Z

packages/github-actions/actions/run-qit-annotate/README.md

+        run: |
+          ./vendor/bin/qit partner:add \
+            --user='${{ secrets.QIT_PARTNER_USER }}' \
+            --application_password='${{ secrets.QIT_PARTNER_SECRET }}'


Noticed this deprecation message on the GL&A side. Looks like this could be changed to use qit_token.

eason9487 · 2024-02-01T02:43:36Z

packages/github-actions/actions/run-qit-extension/action.yml

+      run: |
+        ./vendor/bin/qit partner:add \
+          --user='${{ inputs.qit-partner-user }}' \
+          --application_password='${{ inputs.qit-partner-secret }}'


Same as the above comment.

Addresses #81 (comment)

As it was missing post-review step from #81

Add actions and workflows for QIT tests

90e1a23

tomalec force-pushed the add/qit-workflows branch from bf254a0 to 90e1a23 Compare September 27, 2023 19:59

tomalec added 5 commits September 27, 2023 22:24

Use the pre-released run-qit-annotate action.

764e2d7

Prefix results artifact to avoid conflict with build

ad7a5fa

Forward options input

d8f0673

Support local builds

15d3e40

Run all QIT tests only on demand

cfb8348

remove `on push` used for testing.

tomalec self-assigned this Sep 27, 2023

tomalec added the [actions] changelog: add label Sep 27, 2023

tomalec requested a review from a team September 27, 2023 21:35

tomalec marked this pull request as ready for review September 27, 2023 22:13

tomalec mentioned this pull request Sep 29, 2023

Add QIT workflow woocommerce/google-listings-and-ads#2114

Merged

eason9487 reviewed Oct 2, 2023

View reviewed changes

Fix indentation in run-qit-annotate/README

a8f23ba

Co-authored-by: Eason <eason.su.tw@gmail.com>

Transform the run-qit-extension workflow into an action.

d7e552e

eason9487 reviewed Oct 3, 2023

View reviewed changes

packages/github-actions/actions/run-qit-annotate/action.yml Outdated Show resolved Hide resolved

tomalec added 10 commits January 29, 2024 20:48

Use JSON format of qit command

38a0364

Use changes added in woocommerce/qit-cli#109 Add `ignore_fail` parameter, to simplify continue-on-error behavior. Drop the report file, use annotations only. Hide result URLs, expose `test_run_id` for further checks.

Add wait input to qit action

265e00f

Addresses #81 (review)

Merge branch 'add/qit-workflows-alternative' into add/qit-workflows

af42247

Merge branch 'trunk' into add/qit-workflows

94bd179

Fail GHA when QIT fails (use ignore-fail param if needed)

e93d3c6

Remove results dir from QIT-extensions, add wait param,

44409f5

Update docs.

Update QIT workflow to use qit-extension action

5bdc33d

Make qit action bash permisive to failures, so we can annotate them

5cf8761

Forward qit exitcode to github action

f09dda3

Forward qit exitcode to github action

c592289

Forward ignore fail to qit-extension

c0a8ee1

tomalec requested review from eason9487 and a team January 31, 2024 02:55

eason9487 approved these changes Feb 1, 2024

View reviewed changes

tomalec added 2 commits February 6, 2024 22:53

Add run-qit-extension to the actions README

5f318fb

Addresses #81 (comment)

Use qit_token instead of application_password

d252f0e

Addresses #81 (comment)

tomalec merged commit d360ad6 into trunk Feb 6, 2024

tomalec deleted the add/qit-workflows branch February 6, 2024 22:01

github-actions bot mentioned this pull request Feb 6, 2024

Release version 1.11.0 of the github-actions package #95

Merged

2 tasks

tomalec added a commit that referenced this pull request Feb 6, 2024

Rename add/qit-workflows-test-build to actions-v1 branch

bdaa974

As it was missing post-review step from #81

tomalec mentioned this pull request Feb 6, 2024

Rename add/qit-workflows-test-build to actions-v1 branch #96

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add actions and workflows for QIT tests #81

Add actions and workflows for QIT tests #81

tomalec commented Sep 27, 2023 •

edited

Loading

eason9487 left a comment •

edited

Loading

eason9487 Oct 2, 2023

tomalec Oct 2, 2023

eason9487 Oct 3, 2023 •

edited

Loading

tomalec Oct 18, 2023

eason9487 Oct 27, 2023

eason9487 Oct 27, 2023

tomalec Jan 31, 2024

tomalec commented Oct 2, 2023

tomalec commented Oct 2, 2023

eason9487 commented Oct 3, 2023

tomalec commented Oct 26, 2023

tomalec commented Jan 31, 2024

tomalec commented Jan 31, 2024

eason9487 left a comment

eason9487 Feb 1, 2024

eason9487 Feb 1, 2024

eason9487 Feb 1, 2024

	outputs:
	statuses:
	description: "Statuses of all tests. Array of integers."
	value: ${{ jobs.qit-tests.outputs.statuses }}

Add actions and workflows for QIT tests #81

Add actions and workflows for QIT tests #81

Conversation

tomalec commented Sep 27, 2023 • edited Loading

Changes proposed in this Pull Request:

Screenshots:

Detailed test instructions:

Manual workflow

Reusable action

Additional details:

Changelog entry

eason9487 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eason9487 Oct 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomalec commented Oct 2, 2023

tomalec commented Oct 2, 2023

eason9487 commented Oct 3, 2023

tomalec commented Oct 26, 2023

tomalec commented Jan 31, 2024

tomalec commented Jan 31, 2024

eason9487 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomalec commented Sep 27, 2023 •

edited

Loading

eason9487 left a comment •

edited

Loading

eason9487 Oct 3, 2023 •

edited

Loading