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

Use proxy workflow and commit statuses to record testing results #2143

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

islas
Copy link
Collaborator

@islas islas commented Dec 14, 2024

TYPE: enhancement

KEYWORDS: testing, devops, github, workflow

SOURCE: internal

DESCRIPTION OF CHANGES:
Problem:
The CI/CD testing framework using github actions is set to trigger on PR label events. However, labels are not just used for testing and will trigger the workflow. While the workflow does have checks in place to skip any labels that aren't meant to trigger testing, this skipped workflow status will override any previous actual test status. This can be confusing if two labels are applied at the same time, one being a test label, and only one status appears showing a skipped workflow. Unique naming of workflow runs does not mitigate this problem as the posted status is tied to the workflow internal id and not the run name.

Solution:
Convert the main workflow that hosts the test sets into a dispatch workflow, meaning it must manually be triggered. This has the intended effect of decoupling the workflow id from any PR and will not normally show up as a status at the bottom of PRs solving the issue of independent labels overriding each other. To solve the workflow no longer appearing within PR statuses, the github REST API is used to create a commit status pointing to its respective workflow run via target_url along with the current state of the job.

As the main workflow must manually be triggered, a new entry point proxy workflow is used to filter test labels and request a test run if needed. This paradigm allows events to be triggered within a PR context, simplifying gathering the data necessary to run the correct PR branch. Furthermore, the entry point will still suffer the initial problem of status override on multiple labels, but this should be acceptable as actual test labels will create their own commit statuses once queued.

The dispatch workflow is unable to be run within the context of PR merge refs, nor the head of the branch from a fork (as that would run the workflow in that fork). Thus, the dispatch workflow is run using the base ref of the PR if from a fork, or the head ref ONLY IF originating from the parent repo of the workflow. This means testing of the immediate changes to the workflow can only be observed within the PR of an internal repo branch, limiting development slightly. The benefits, however, are a cleaned up status reporting AND increased security as no runner code that isn't already within a branch of the repository will be executed. One should still ensure the underlying tests are okay to run

TESTS CONDUCTED:

  1. Testing was done in independent fork to demonstrate initial issue and feasibility of this solution.

This will effectively decouple the trigger events of a PR from this
PR running, thereby avoiding either superfluous statuses of skipped
jobs or overriden statuses from actual jobs from multiple label
events. Updates are made to use the inputs provided by the dispatch
trigger rather than now non-existent context variables.
The input trigger or default push shall inform which
SHA to use, which will effectively reinform the PR of
the state of the workflow run. This now allows the
workflow to deliberately override or post workflow
runs to a PR without implicit events removing previous
statuses or skipped jobs polluting the PR state.
…flow

This simple entry point workflow should funnel event webhooks
to the dispatch workflow IF the event is deemed an actual test
event. This combined with the decoupled dispatch workflow is
how PRs will still be able to trigger testing without needing
manual running of the dispatch workflow. Likewise, it will stop
unnecessary skipped job statuses from showing up on the PR.

The status of this workflow will inevitably be constantly
overriden from multiple label events, but the published
commit status of the dispatch workflow, if queued, will
provide the handle to the run via a target_url.

Since the workflow_dispatch webhook event cannot take a
pull request merge ref, certain measures must be taken to
allow running the workflow in a valid context. For PRs
originating from forks, it is impossible to run the workflow
in the parent repo using the head ref as that branch exists
in a different repo. In this case we will run the base ref
of the PR. This has the added benefit of security, ensuring
the runners never run workflows from outside sources.
Unfortunately, this would limit critical patches to the
workflow from being demonstated within a PR. To allow for
this use case, when a PR from an internal repo branch is
used, the dispatch event will be made using the PR branch
head ref, thus running the changes.

This paradigm will also guarantee that the workflow_dispatch
will only ever use internal refs, increasing security and
allowing label modifications.
@islas islas requested a review from a team as a code owner December 14, 2024 00:34
@islas
Copy link
Collaborator Author

islas commented Dec 14, 2024

As noted in the PR description, only internal branches can be used to immediately see workflow changes in effect. That is why this PR is coming from actions_testing

Further detail on this strategy can be read in 91f918b

Examples of this running can also be found in islas#10 to see a mock test failure

@weiwangncar
Copy link
Collaborator

The Jenkins test results:

Test Type              | Expected  | Received |  Failed
= = = = = = = = = = = = = = = = = = = = = = = =  = = = =
Number of Tests        : 23           24
Number of Builds       : 60           57
Number of Simulations  : 158           150        0
Number of Comparisons  : 95           86        0

Failed Simulations are: 
None
Which comparisons are not bit-for-bit: 
None

@amstokely amstokely self-requested a review December 16, 2024 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants