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

fix: updating branch name fallbacks for GitHub Actions recordings #27409

Merged
merged 5 commits into from
Jul 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@

_Released 08/01/2023 (PENDING)_

**Bugfixes:**

- Fixed an issue where unexpected branch names were being recorded for cypress runs when executed by GitHub Actions. The HEAD branch name will now be recorded by default for pull request workflows if a branch name cannot otherwise be detected from user overrides or from local git data. Fixes [#27389](https://github.com/cypress-io/cypress/issues/27389).

**Performance:**

- Fixed an issue where unnecessary requests were being paused. No longer sends `X-Cypress-Is-XHR-Or-Fetch` header and infers resource type off of the server pre-request object. Fixes [#26620](https://github.com/cypress-io/cypress/issues/26620) and [#26622](https://github.com/cypress-io/cypress/issues/26622).
Expand Down
12 changes: 11 additions & 1 deletion packages/server/lib/util/ci_provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,10 @@ const _providerCiParams = () => {
'GITHUB_RUN_ID',
'GITHUB_RUN_ATTEMPT',
'GITHUB_REPOSITORY',
'GITHUB_BASE_REF',
'GITHUB_HEAD_REF',
'GITHUB_REF_NAME',
'GITHUB_REF',
Copy link
Contributor Author

@tbiethman tbiethman Jul 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I was in here, I added the GHA variables we use for commit fallbacks (+ GITHUB_REF for completeness) to the ci_params mapping. These additions aren't necessary to correct the current fallback values, but they may be useful in the future for further inspection of the recordings.

]),
// see https://docs.gitlab.com/ee/ci/variables/
gitlab: extract([
Expand Down Expand Up @@ -536,7 +540,13 @@ const _providerCommitParams = () => {
},
githubActions: {
sha: env.GITHUB_SHA,
branch: env.GH_BRANCH || env.GITHUB_REF,
// GH_BRANCH - populated with HEAD branch by cypress/github-action
// GITHUB_HEAD_REF - populated with the head ref or source branch
// of the pull request in a workflow run and is
// otherwise unset
// GITHUB_REF_NAME - populated with short ref name of the branch or
// tag that triggered the workflow run
branch: env.GH_BRANCH || env.GITHUB_HEAD_REF || env.GITHUB_REF_NAME,
defaultBranch: env.GITHUB_BASE_REF,
remoteBranch: env.GITHUB_HEAD_REF,
runAttempt: env.GITHUB_RUN_ATTEMPT,
Expand Down
37 changes: 27 additions & 10 deletions packages/server/test/unit/util/ci_provider_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -561,23 +561,21 @@ describe('lib/util/ci_provider', () => {
})

it('github actions', () => {
// with GH_BRANCH used as branch
resetEnv = mockedEnv({
GITHUB_ACTIONS: 'true',

GITHUB_WORKFLOW: 'ciGitHubWorkflowName',
GITHUB_ACTION: 'ciGitHubActionId',
GITHUB_EVENT_NAME: 'ciEventName',
GITHUB_RUN_ID: 'ciGithubRunId',
GITHUB_RUN_ATTEMPT: 'ciGithubRunAttempt',
GITHUB_REPOSITORY: 'ciGithubRepository',
GH_BRANCH: '',

GITHUB_SHA: 'ciCommitSha',
GH_BRANCH: 'GHCommitBranch',
GITHUB_REF: 'ciCommitRef',

// only for forked repos
GITHUB_HEAD_REF: 'ciHeadRef',
GITHUB_BASE_REF: 'ciBaseRef',
GITHUB_REF_NAME: 'ciRefName',
}, { clear: true })

expectsName('githubActions')
Expand All @@ -588,26 +586,45 @@ describe('lib/util/ci_provider', () => {
githubRepository: 'ciGithubRepository',
githubRunAttempt: 'ciGithubRunAttempt',
githubRunId: 'ciGithubRunId',
githubBaseRef: 'ciBaseRef',
githubHeadRef: 'ciHeadRef',
githubRefName: 'ciRefName',
githubRef: 'ciCommitRef',
})

expectsCommitParams({
sha: 'ciCommitSha',
defaultBranch: 'ciBaseRef',
runAttempt: 'ciGithubRunAttempt',
remoteBranch: 'ciHeadRef',
branch: 'ciCommitRef',
branch: 'GHCommitBranch',
})

// with GITHUB_HEAD_REF used as branch
resetEnv = mockedEnv({
GITHUB_ACTIONS: 'true',
GH_BRANCH: undefined,
GITHUB_HEAD_REF: 'ciHeadRef',
GITHUB_REF_NAME: 'ciRefName',
GITHUB_REF: 'ciCommitRef',
}, { clear: true })

expectsCommitParams({
branch: 'ciHeadRef',
remoteBranch: 'ciHeadRef',
})

// with GITHUB_REF_NAME used as branch
resetEnv = mockedEnv({
GITHUB_ACTIONS: 'true',
GH_BRANCH: undefined,
GITHUB_HEAD_REF: undefined,
GITHUB_REF_NAME: 'ciRefName',
GITHUB_REF: 'ciCommitRef',
GH_BRANCH: 'GHCommitBranch',
GITHUB_RUN_ATTEMPT: 'ciGithubRunAttempt',
}, { clear: true })

return expectsCommitParams({
branch: 'GHCommitBranch',
runAttempt: 'ciGithubRunAttempt',
branch: 'ciRefName',
})
})

Expand Down
4 changes: 2 additions & 2 deletions packages/telemetry/src/detectors/githubActionsDetectorSync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ class GithubActionsDetectorSync implements DetectorSync {
detect (): IResource {
const attributes: ResourceAttributes = {}

const { GITHUB_ACTION, GH_BRANCH, GITHUB_REF, GITHUB_SHA, GITHUB_RUN_NUMBER } = process.env
const { GITHUB_ACTION, GH_BRANCH, GITHUB_HEAD_REF, GITHUB_REF_NAME, GITHUB_SHA, GITHUB_RUN_NUMBER } = process.env

if (GITHUB_ACTION) {
attributes['ci.github_action'] = GITHUB_ACTION
attributes['ci.build-number'] = GITHUB_RUN_NUMBER
attributes['ci.branch'] = GH_BRANCH || GITHUB_REF
attributes['ci.branch'] = GH_BRANCH || GITHUB_HEAD_REF || GITHUB_REF_NAME
attributes['SHA1'] = GITHUB_SHA
}

Expand Down
36 changes: 25 additions & 11 deletions packages/telemetry/test/detectors/githubActionsDetectorSync.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,16 @@ describe('githubActionsDetectorSync', () => {
// cache values
processValues.GITHUB_ACTION = process.env.GITHUB_ACTION
processValues.GH_BRANCH = process.env.GH_BRANCH
processValues.GITHUB_REF = process.env.GITHUB_REF
processValues.GITHUB_HEAD_REF = process.env.GITHUB_HEAD_REF
processValues.GITHUB_REF_NAME = process.env.GITHUB_REF_NAME
processValues.GITHUB_RUN_NUMBER = process.env.GITHUB_RUN_NUMBER
processValues.GITHUB_SHA = process.env.GITHUB_SHA

//reset values
delete process.env.GITHUB_ACTION
delete process.env.GH_BRANCH
delete process.env.GITHUB_REF
delete process.env.GITHUB_HEAD_REF
delete process.env.GITHUB_REF_NAME
delete process.env.GITHUB_RUN_NUMBER
delete process.env.GITHUB_SHA
})
Expand All @@ -26,7 +28,8 @@ describe('githubActionsDetectorSync', () => {
// Replace values
process.env.GITHUB_ACTION = processValues.GITHUB_ACTION
process.env.GH_BRANCH = processValues.GH_BRANCH
process.env.GITHUB_REF = processValues.GITHUB_REF
process.env.GITHUB_HEAD_REF = processValues.GITHUB_HEAD_REF
process.env.GITHUB_REF_NAME = processValues.GITHUB_REF_NAME
process.env.GITHUB_RUN_NUMBER = processValues.GITHUB_RUN_NUMBER
process.env.GITHUB_SHA = processValues.GITHUB_SHA
})
Expand All @@ -47,14 +50,16 @@ describe('githubActionsDetectorSync', () => {
// cache values
processValues.GITHUB_ACTION = process.env.GITHUB_ACTION
processValues.GH_BRANCH = process.env.GH_BRANCH
processValues.GITHUB_REF = process.env.GITHUB_REF
processValues.GITHUB_HEAD_REF = process.env.GITHUB_HEAD_REF
processValues.GITHUB_REF_NAME = process.env.GITHUB_REF_NAME
processValues.GITHUB_RUN_NUMBER = process.env.GITHUB_RUN_NUMBER
processValues.GITHUB_SHA = process.env.GITHUB_SHA

//reset values
process.env.GITHUB_ACTION = 'githubAction'
process.env.GH_BRANCH = 'ghBranch'
process.env.GITHUB_REF = 'ghRef'
process.env.GITHUB_HEAD_REF = 'ghHeadRef'
process.env.GITHUB_REF_NAME = 'ghRefName'
process.env.GITHUB_RUN_NUMBER = 'ghRunNumber'
process.env.GITHUB_SHA = 'ghSha'
})
Expand All @@ -63,7 +68,8 @@ describe('githubActionsDetectorSync', () => {
// Replace values
process.env.GITHUB_ACTION = processValues.GITHUB_ACTION
process.env.GH_BRANCH = processValues.GH_BRANCH
process.env.GITHUB_REF = processValues.GITHUB_REF
process.env.GITHUB_HEAD_REF = processValues.GITHUB_HEAD_REF
process.env.GITHUB_REF_NAME = processValues.GITHUB_REF_NAME
process.env.GITHUB_RUN_NUMBER = processValues.GITHUB_RUN_NUMBER
process.env.GITHUB_SHA = processValues.GITHUB_SHA
})
Expand All @@ -72,23 +78,31 @@ describe('githubActionsDetectorSync', () => {
it('returns a resource with attributes', () => {
const resource = githubActionsDetectorSync.detect()

console.log(resource.attributes)

expect(resource.attributes['ci.github_action']).to.equal('githubAction')
expect(resource.attributes['ci.branch']).to.equal('ghBranch')
expect(resource.attributes['ci.build-number']).to.equal('ghRunNumber')
expect(resource.attributes['SHA1']).to.equal('ghSha')
})

it('returns a resource with attributes when gh_branch is missing', () => {
it('returns a resource with attributes when GH_BRANCH is missing', () => {
delete process.env.GH_BRANCH

const resource = githubActionsDetectorSync.detect()

console.log(resource.attributes)
expect(resource.attributes['ci.github_action']).to.equal('githubAction')
expect(resource.attributes['ci.branch']).to.equal('ghHeadRef')
expect(resource.attributes['ci.build-number']).to.equal('ghRunNumber')
expect(resource.attributes['SHA1']).to.equal('ghSha')
})

it('returns a resource with attributes when GH_BRANCH and GITHUB_HEAD_REF is missing', () => {
delete process.env.GH_BRANCH
delete process.env.GITHUB_HEAD_REF

const resource = githubActionsDetectorSync.detect()

expect(resource.attributes['ci.github_action']).to.equal('githubAction')
expect(resource.attributes['ci.branch']).to.equal('ghRef')
expect(resource.attributes['ci.branch']).to.equal('ghRefName')
expect(resource.attributes['ci.build-number']).to.equal('ghRunNumber')
expect(resource.attributes['SHA1']).to.equal('ghSha')
})
Expand Down