feat: [CYCLOUD-1447] Auto-detect PR numbers #1009

ax-vasquez · 2023-08-28T18:14:53Z

The Cypress GitHub action does not currently expose PR information in a way that is immediately-usable by the app (without further processing).

This PR updates the GitHub action so that it detects the PR number and passes it along in a way that can be consumed by the app. Currently we are targeting the existing environment variables, CYPRESS_PULL_REQUEST_ID & CYPRESS_PULL_REQUEST_URL since both of these already exist and are currently in-use by both the app and cloud.

NOTE: If the user has already set these variables, we will not overwrite them.

PR Number detection logic

When the GitHub event provides us with the PR info, we simply pass it along
If the event doesn't have PR info, we instead grab the first PR related to the commit using GitHub's listPullRequestsAssociatedWithCommit API

Testing

For each of these test scenarios, start with the following steps:

Set up a project with the Cypress GitHub action

Customize your action so that it echos the CYPRESS_PULL_REQUEST_ID & CYPRESS_PULL_REQUEST_URL values

With PR

Open a PR > push a commit and trigger the action
Check the action output and ensure the echo-ed values match that of the PR from which the action was triggered

With no direct PR, but with related PRs

Create a new branch from a branch that has a PR already
Push the branch up to GitHub, but do not create a PR for the new branch
Push the branch, but don't create a new commit (this new commit won't exist in the base PR if you do) > this should trigger a run
Check the action output and ensure the echo-ed values match that of the PR from the base branch

With no PRs at all

Close all open PRs
Create a new branch
Push a commit to trigger a new run
Check the action output and ensure the echo-ed values are empty

CLAassistant · 2023-08-28T18:14:59Z

All committers have signed the CLA.

index.js

MikeMcC399 · 2023-08-28T18:42:40Z

@ax-vasquez

Will you add a detailed description of what this PR is intended to achieve and how you will test it?

You should read the CONTRIBUTING.md and apply the guidance in it.

Note that this repo is based on npm not Yarn, so you shouldn't have a yarn.lock in your PR. The repo uses package-lock.json.

ax-vasquez · 2023-08-30T16:19:52Z

@ax-vasquez

Will you add a detailed description of what this PR is intended to achieve and how you will test it?

You should read the CONTRIBUTING.md and apply the guidance in it.

Note that this repo is based on npm not Yarn, so you shouldn't have a yarn.lock in your PR. The repo uses package-lock.json.

Yes, I will be sure to do that once this I have this in a state that's ready for review.

MikeMcC399 · 2023-08-30T17:07:48Z

@ax-vasquez

Thanks very much for the description! Probably a shorter version of that should go into the README at some stage.

You need to either enable HUSKY hooks or run

npm ci
npm run format
npm run build

to make sure that the code that you've written is actually the code which gets executed.

I'll hold back on making any more comments unless you ask for review, since this is still a Draft PR.

It looks like this PR would resolve No way to manually provide pull_request information? #350

ax-vasquez · 2023-08-30T17:09:37Z

@MikeMcC399 no problem! I was still working out some of the details when I had been pulled to something else; apologies for the delay there.

Once we're certain this is working as we intend, the last part of this task is to update the docs with the changes, so I will be sure to update relevant docs before merging this one 👍

Appreciate the feedback here!

…rmando/fix/CYCLOUD-1447

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

MikeMcC399 · 2023-08-31T17:03:41Z

Hi @tbiethman 👋🏻

I have a release-related question for you. I see we have major versions v5 and v6 for the action, with v5 our current recommended version. Which major version would these changes be released under using the semantic release bot? Releasing under both would be ideal.

We're in a transition phase right now because GitHub hasn't properly documented that node20 is supported. That's why I hadn't changed the recommended version to v6 yet.

We can't use v5 to add any new features because this is now frozen. The sematic release workflow is not set up to maintain different versions at the same time and that would require quite some effort to implement without any real benefit.

I will accelerate moving to v6 as the recommended version. I already have that prepared, so it is just a question of submitting it as a PR. You can then ignore v5 and we'll just refer to v6 going forward. Stand by 🙂 !

MikeMcC399 · 2023-08-31T17:11:32Z

@tbiethman

feat: recommend v6 as standard action version #1014 bumps the recommended action version to v6

Co-authored-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>

MikeMcC399 · 2023-08-31T17:32:51Z

@tbiethman

The Cypress Cloud template for GitHub actions should also be updated in conjunction with this PR. Will you handle that process? If not, I would open a separate issue for that:

Open https://cloud.cypress.io/
Click "New project"
Enter a project name, click "Create project"
Under section 3 "Finish CI setup", click GitHub Actions

tbiethman · 2023-08-31T17:43:35Z

@MikeMcC399 good call, I'll work on getting those updated cloud-side.

tbiethman · 2023-08-31T17:56:57Z

index.js

+      // should have format refs/pull/<pr_number>/merge when triggered by pull_request workflow
+      prNumber = parseInt(GITHUB_REF.split('/')[2])
+    } else {
+      const resp = await client.request(


Might also be worth adding a try/catch around this request in the event of a partial outage or something. I don't think we'd want to halt a recording by throwing in that case.

tbiethman · 2023-08-31T18:08:43Z

README.md

+      GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+```
+
+#### Branch with PR


Rather than focusing these sections around branches/PRs, it might make more sense to specify the behavior based on the various workflow event types.

For example, if the action executes on a push event, we'll still need to make the request, even if there is a PR open, as the GITHUB_HEAD_REF env will not be set for that event.

We could do something like:

<example yaml here> The behavior of the action varies based on the triggering event type. #### Triggering event: `pull_request`/`pull_request_target` ... #### Triggering event: `push` ...

Good call! I'll update this 👍 That's much clearer.

tbiethman

Left a few last minute comments. My testing has shown this all to be working great!

README.md

MikeMcC399 · 2023-09-01T14:10:28Z

@tbiethman

@v6 is now the recommended version, so you can disregard @v5 now

Co-authored-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>

tbiethman

Ran through my tests on the latest commit and all works as expected!

README.md

ax-vasquez · 2023-09-01T17:13:26Z

@MikeMcC399 we've had one approval so far - just wanted to get the thumbs up from you as well before we merge. Let me know if this is good with you! 😃

Co-authored-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>

MikeMcC399

Just one small documentation suggestion, otherwise LGTM! 👍🏻

ax-vasquez · 2023-09-01T17:14:30Z

Awesome, thanks! I committed the change - will merge this on green 👍

github-actions · 2023-09-01T17:28:44Z

🎉 This PR is included in version 6.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

WIP - need to ask for ideal way to check existing variables

ade6071

ax-vasquez commented Aug 28, 2023

View reviewed changes

index.js Outdated Show resolved Hide resolved

ax-vasquez added 2 commits August 28, 2023 13:54

check for existing variables

afef3a6

consolidate comments

2e6067c

ax-vasquez added 2 commits August 30, 2023 11:54

remove yarn lock file

d83963a

Merge branch 'master' into armando/fix/CYCLOUD-1447

e277f50

ax-vasquez and others added 19 commits August 30, 2023 12:13

run commands from PR feedback

6a7498b

Merge remote-tracking branch 'origin/armando/fix/CYCLOUD-1447' into a…

1ccad80

…rmando/fix/CYCLOUD-1447

Merge branch 'master' into armando/fix/CYCLOUD-1447

12dcc6e

logging for testing purposes only

adebbb7

Merge remote-tracking branch 'origin/armando/fix/CYCLOUD-1447' into a…

19380ba

…rmando/fix/CYCLOUD-1447

more logging

9051d08

more logging

2e38a45

simplify URL logic

89fe794

more logging

9321be8

remove logging - fix url string

3a17a1f

log response for fallback logic

d984062

docs: rewrite readme working directory description (#1005)

d2df9c5

chore(deps): update dependency @types/node to v20.5.7 (#1008)

2aceb30

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

docs: rework requirements for contributors (#1010)

34055c5

feat(deps): update cypress to 13.0.0 (#1012)

a4ecbdd

logging for testing purposes only

44670f1

test: run component test example from current branch (#1013)

a28c1f3

more logging

b32c154

more logging

f9b9475

MikeMcC399 mentioned this pull request Aug 31, 2023

feat: recommend v6 as standard action version #1014

Merged

Update README.md

620df0d

Co-authored-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>

tbiethman reviewed Aug 31, 2023

View reviewed changes

MikeMcC399 reviewed Sep 1, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

ax-vasquez and others added 4 commits September 1, 2023 11:20

Update README.md

74dbf08

Co-authored-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>

try catch pr data request

d108631

Readme update

7896153

Merge branch 'master' into armando/fix/CYCLOUD-1447

0a94c6d

ax-vasquez requested review from tbiethman and MikeMcC399 September 1, 2023 16:53

tbiethman approved these changes Sep 1, 2023

View reviewed changes

MikeMcC399 reviewed Sep 1, 2023

View reviewed changes

README.md Outdated Show resolved Hide resolved

Update README.md

f8f90f7

Co-authored-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>

MikeMcC399 suggested changes Sep 1, 2023

View reviewed changes

ax-vasquez requested a review from MikeMcC399 September 1, 2023 17:14

MikeMcC399 approved these changes Sep 1, 2023

View reviewed changes

ax-vasquez merged commit 7215fc1 into master Sep 1, 2023
69 checks passed

ax-vasquez deleted the armando/fix/CYCLOUD-1447 branch September 1, 2023 17:27

github-actions bot added the released label Sep 1, 2023

MikeMcC399 mentioned this pull request Sep 1, 2023

docs: add changelog for 6.3.0 and 6.4.0 #1016

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [CYCLOUD-1447] Auto-detect PR numbers #1009

feat: [CYCLOUD-1447] Auto-detect PR numbers #1009

ax-vasquez commented Aug 28, 2023 •

edited

CLAassistant commented Aug 28, 2023 •

edited

MikeMcC399 commented Aug 28, 2023

ax-vasquez commented Aug 30, 2023

MikeMcC399 commented Aug 30, 2023

ax-vasquez commented Aug 30, 2023

MikeMcC399 commented Aug 31, 2023

MikeMcC399 commented Aug 31, 2023

MikeMcC399 commented Aug 31, 2023

tbiethman commented Aug 31, 2023

tbiethman Aug 31, 2023 •

edited

tbiethman Aug 31, 2023

ax-vasquez Sep 1, 2023

tbiethman left a comment

MikeMcC399 commented Sep 1, 2023

tbiethman left a comment

ax-vasquez commented Sep 1, 2023

MikeMcC399 left a comment

ax-vasquez commented Sep 1, 2023

github-actions bot commented Sep 1, 2023

feat: [CYCLOUD-1447] Auto-detect PR numbers #1009

feat: [CYCLOUD-1447] Auto-detect PR numbers #1009

Conversation

ax-vasquez commented Aug 28, 2023 • edited

PR Number detection logic

Testing

CLAassistant commented Aug 28, 2023 • edited

MikeMcC399 commented Aug 28, 2023

ax-vasquez commented Aug 30, 2023

MikeMcC399 commented Aug 30, 2023

ax-vasquez commented Aug 30, 2023

MikeMcC399 commented Aug 31, 2023

MikeMcC399 commented Aug 31, 2023

MikeMcC399 commented Aug 31, 2023

tbiethman commented Aug 31, 2023

tbiethman Aug 31, 2023 • edited

Choose a reason for hiding this comment

tbiethman Aug 31, 2023

Choose a reason for hiding this comment

ax-vasquez Sep 1, 2023

Choose a reason for hiding this comment

tbiethman left a comment

Choose a reason for hiding this comment

MikeMcC399 commented Sep 1, 2023

tbiethman left a comment

Choose a reason for hiding this comment

ax-vasquez commented Sep 1, 2023

MikeMcC399 left a comment

Choose a reason for hiding this comment

ax-vasquez commented Sep 1, 2023

github-actions bot commented Sep 1, 2023

ax-vasquez commented Aug 28, 2023 •

edited

CLAassistant commented Aug 28, 2023 •

edited

tbiethman Aug 31, 2023 •

edited