Run browser tests on forked PRs by a dedicated label

[outsideris]

Description of the Change

It is the only way I found to run browser tests on forked PR.

After reviewing code of a forked PR, maintainers can attach a run-browser-test label.
Maintainers should check there is no malicious code to steal our secrets or do something bad on our repository.

This Browser Tests workflow will run based on forked PRs with writing repo and access secrets permissions. So, it can be dangerous if the forked PR has malicious code.

I've tested pull_request_target event.

When users open a pull request, push, pull_reqeust and pull_request_target will be run automatically. But pull_request_target: types: [labeled](in this case Browser Tests) will not be run because labeled type.

If maintainers attach a label except run-browser-test label, Browser Tests workflow will be triggered, but it skipped because the label is not run-browser-test. In the above screenshot, pull request safe run workflow is pull_request_target workflow with labeled type.

When maintainers attach the run-browser-test label, Browser Tests workflow will be triggered. In above screenshot, I tested it with safe-to-run label.

After that, when author of the forked PR pushed new commits, only default workflows will. be run.

That means maintainers should remove and re-attache run-browser-test label to run Browser Tests again.

And when the forked PR already has run-browser-test label, Browser Tests workflow will be triggered if maintainers attach a new label except run-browser-test label. We have to be careful with this.

Alternate Designs

We can use workflow_dispatch event. workflow_dispatch is a workflow that we should trigger it manually with PR number.

I think there are some cons.

• Process is not convenient. maintainers enter PR number and trigger the workflow manually.
• It couldn't be integrated with PR. So, we cannot the result of the workflow in GitHub's checks tab. It is hard to know whether browser tests are successful or fail.

Therefore, I think pull_request_target event with labeled type is better than workflow_dispatch event.

Why should this be in core?

All forked PRs failed because of browser tests.

Applicable issues

close #4553

[coveralls]

Coverage remained the same at 94.201% when pulling af8c9c6 on outsideris:forked-pr-browser-test into d7ed5c2 on mochajs:master.

[juergba]

When users open a PR, push, pull_request and pull_request_target will be run automatically.

There shouldn't be any push event for users = forked PRs, right?

Your solution is awkward, but I don't see any alternative way either...

You could remove the run-browser-test label automatically after browser test's completion.
There is a GH action re-run-workflow which:
once-label: When this label is added to a pull request, re-run the workflow once and remove the label again.
What do you think about above GH re-run-workflow?
Or any other app which removes labels (search the marketplace with "remove labels").

[outsideris]

There shouldn't be any push event for users = forked PRs, right?

No, it shouldn't be triggered by forked PRs. I just mentioned push, pull_request and pull_request_target because I want to explain normal workflows like our smoke, lint and tests.

You could remove the run-browser-test label automatically after browser test's completion.

That's a good idea. I will try it.

[outsideris]

I used Add Remove Label actions.

It will remove run-browser-test label after browser test workflow whether it succeeds or fails.

And I will more test with real PRs after merging this because there are some limitations outside of this repository.

[juergba]

I have some small comments only, lgtm.

[outsideris]

fixed.

I will merge this to test with real forked PRs.