diff --git a/src/api/core.js b/src/api/core.js index 78b895f4..1d4df56b 100644 --- a/src/api/core.js +++ b/src/api/core.js @@ -159,6 +159,14 @@ export function buildBuildPayload(options) { payload.commit_message = options.message || options.commit_message; } + if (options.commit_author_name) { + payload.commit_author_name = options.commit_author_name; + } + + if (options.commit_author_email) { + payload.commit_author_email = options.commit_author_email; + } + if (options.pullRequestNumber || options.github_pull_request_number) { payload.github_pull_request_number = options.pullRequestNumber || options.github_pull_request_number; diff --git a/src/commands/run.js b/src/commands/run.js index 3139ddf6..a2a1c83b 100644 --- a/src/commands/run.js +++ b/src/commands/run.js @@ -30,6 +30,7 @@ import { loadConfig as defaultLoadConfig } from '../utils/config-loader.js'; import { detectBranch as defaultDetectBranch, detectCommit as defaultDetectCommit, + detectCommitAuthor as defaultDetectCommitAuthor, detectCommitMessage as defaultDetectCommitMessage, detectPullRequestNumber as defaultDetectPullRequestNumber, generateBuildNameWithGit as defaultGenerateBuildNameWithGit, @@ -119,6 +120,7 @@ export async function runCommand( runTests = defaultRunTests, detectBranch = defaultDetectBranch, detectCommit = defaultDetectCommit, + detectCommitAuthor = defaultDetectCommitAuthor, detectCommitMessage = defaultDetectCommitMessage, detectPullRequestNumber = defaultDetectPullRequestNumber, generateBuildNameWithGit = defaultGenerateBuildNameWithGit, @@ -245,6 +247,7 @@ export async function runCommand( let commit = await detectCommit(options.commit || config.build.commit); let message = options.message || config.build.message || (await detectCommitMessage()); + let commitAuthor = await detectCommitAuthor(); let buildName = await generateBuildNameWithGit( options.buildName || configuredBuildName ); @@ -306,6 +309,8 @@ export async function runCommand( branch, commit, message, + commitAuthorName: commitAuthor.name, + commitAuthorEmail: commitAuthor.email, environment: config.build.environment, threshold: config.comparison.threshold, minClusterSize: config.comparison.minClusterSize, diff --git a/src/commands/tdd.js b/src/commands/tdd.js index 161a0c65..313bee79 100644 --- a/src/commands/tdd.js +++ b/src/commands/tdd.js @@ -25,6 +25,7 @@ import { loadConfig as defaultLoadConfig } from '../utils/config-loader.js'; import { detectBranch as defaultDetectBranch, detectCommit as defaultDetectCommit, + detectCommitAuthor as defaultDetectCommitAuthor, detectCommitMessage as defaultDetectCommitMessage, detectPullRequestNumber as defaultDetectPullRequestNumber, generateBuildNameWithGit as defaultGenerateBuildNameWithGit, @@ -75,6 +76,7 @@ export async function tddCommand( runTests = defaultRunTests, detectBranch = defaultDetectBranch, detectCommit = defaultDetectCommit, + detectCommitAuthor = defaultDetectCommitAuthor, detectCommitMessage = defaultDetectCommitMessage, detectPullRequestNumber = defaultDetectPullRequestNumber, generateBuildNameWithGit = defaultGenerateBuildNameWithGit, @@ -136,6 +138,7 @@ export async function tddCommand( let commit = await detectCommit(options.commit || config.build.commit); let message = options.message || config.build.message || (await detectCommitMessage()); + let commitAuthor = await detectCommitAuthor(); let buildName = await generateBuildNameWithGit( options.buildName || configuredBuildName ); @@ -191,6 +194,8 @@ export async function tddCommand( branch, commit, message, + commitAuthorName: commitAuthor.name, + commitAuthorEmail: commitAuthor.email, environment: config.build.environment, pullRequestNumber, parallelId: config.parallelId, diff --git a/src/commands/upload.js b/src/commands/upload.js index 49c1643b..35011c83 100644 --- a/src/commands/upload.js +++ b/src/commands/upload.js @@ -10,6 +10,7 @@ import { loadConfig as defaultLoadConfig } from '../utils/config-loader.js'; import { detectBranch as defaultDetectBranch, detectCommit as defaultDetectCommit, + detectCommitAuthor as defaultDetectCommitAuthor, detectCommitMessage as defaultDetectCommitMessage, detectPullRequestNumber as defaultDetectPullRequestNumber, generateBuildNameWithGit as defaultGenerateBuildNameWithGit, @@ -77,6 +78,7 @@ export async function uploadCommand( createUploader = defaultCreateUploader, detectBranch = defaultDetectBranch, detectCommit = defaultDetectCommit, + detectCommitAuthor = defaultDetectCommitAuthor, detectCommitMessage = defaultDetectCommitMessage, detectPullRequestNumber = defaultDetectPullRequestNumber, generateBuildNameWithGit = defaultGenerateBuildNameWithGit, @@ -121,6 +123,7 @@ export async function uploadCommand( let commit = await detectCommit(options.commit || config.build.commit); let message = options.message || config.build.message || (await detectCommitMessage()); + let commitAuthor = await detectCommitAuthor(); let buildName = await generateBuildNameWithGit( options.buildName || configuredBuildName ); @@ -148,6 +151,8 @@ export async function uploadCommand( branch, commit, message, + commitAuthorName: commitAuthor.name, + commitAuthorEmail: commitAuthor.email, environment: config.build.environment, threshold: config.comparison.threshold, minClusterSize: config.comparison.minClusterSize, diff --git a/src/test-runner/core.js b/src/test-runner/core.js index bde0a3a5..e46fc4c3 100644 --- a/src/test-runner/core.js +++ b/src/test-runner/core.js @@ -64,6 +64,8 @@ export function buildApiBuildPayload(options, comparisonConfig = null) { environment: options.environment || 'test', commit_sha: options.commit, commit_message: options.message, + commit_author_name: options.commitAuthorName, + commit_author_email: options.commitAuthorEmail, github_pull_request_number: options.pullRequestNumber, parallel_id: options.parallelId, }; diff --git a/src/types/index.d.ts b/src/types/index.d.ts index 629d3da5..923b7f80 100644 --- a/src/types/index.d.ts +++ b/src/types/index.d.ts @@ -165,6 +165,8 @@ export interface UploadOptions { branch?: string; commit?: string; message?: string; + commitAuthorName?: string; + commitAuthorEmail?: string; environment?: string; threshold?: number; minClusterSize?: number; @@ -517,6 +519,8 @@ export interface BuildOptions { commit_sha?: string; message?: string; commit_message?: string; + commitAuthorName?: string; + commitAuthorEmail?: string; environment?: string; threshold?: number; eager?: boolean; diff --git a/src/uploader/core.js b/src/uploader/core.js index 81cb5d53..dc032097 100644 --- a/src/uploader/core.js +++ b/src/uploader/core.js @@ -132,6 +132,8 @@ export function buildBuildInfo(options, defaultBranch = 'main') { branch: options.branch || defaultBranch || 'main', commit_sha: options.commit, commit_message: options.message, + commit_author_name: options.commitAuthorName, + commit_author_email: options.commitAuthorEmail, environment: options.environment || 'production', threshold: options.threshold, github_pull_request_number: options.pullRequestNumber, diff --git a/src/utils/ci-env.js b/src/utils/ci-env.js index 9535754e..02681ce7 100644 --- a/src/utils/ci-env.js +++ b/src/utils/ci-env.js @@ -131,12 +131,25 @@ export function getCommit() { } /** - * Get the commit message from CI environment variables + * Get the commit message from CI environment variables. + * + * For GitHub Actions pull_request events, the checkout is a synthetic merge + * commit whose message is "Merge into ", so the PR title from the + * event payload is used instead. + * * @returns {string|null} Commit message or null if not available */ export function getCommitMessage() { + if (process.env.VIZZLY_COMMIT_MESSAGE) { + return process.env.VIZZLY_COMMIT_MESSAGE; + } + + if (process.env.GITHUB_ACTIONS) { + let title = getGitHubEvent().pull_request?.title; + if (title) return title; + } + return ( - process.env.VIZZLY_COMMIT_MESSAGE || // Vizzly override process.env.CI_COMMIT_MESSAGE || // GitLab CI process.env.TRAVIS_COMMIT_MESSAGE || // Travis CI process.env.BUILDKITE_MESSAGE || // Buildkite @@ -147,6 +160,30 @@ export function getCommitMessage() { ); } +/** + * Parse a "Name " author string (GitLab's CI_COMMIT_AUTHOR format) + * @param {string|undefined} value - Author string + * @returns {{ name: string|null, email: string|null }} + */ +function parseAuthorString(value) { + let match = value?.match(/^(.*?)\s*<([^>]*)>\s*$/); + if (!match) return { name: value?.trim() || null, email: null }; + return { name: match[1] || null, email: match[2] || null }; +} + +/** + * Get the commit author from CI environment variables + * @returns {{ name: string|null, email: string|null }} Author name and email + */ +export function getCommitAuthor() { + let gitlab = parseAuthorString(process.env.CI_COMMIT_AUTHOR); + + return { + name: process.env.VIZZLY_COMMIT_AUTHOR_NAME || gitlab.name || null, + email: process.env.VIZZLY_COMMIT_AUTHOR_EMAIL || gitlab.email || null, + }; +} + function parsePullRequestNumber(value) { if (!/^\d+$/.test(value)) { return null; diff --git a/src/utils/git.js b/src/utils/git.js index 82b12771..cd4e475a 100644 --- a/src/utils/git.js +++ b/src/utils/git.js @@ -3,6 +3,7 @@ import { promisify } from 'node:util'; import { getBranch as getCIBranch, getCommit as getCICommit, + getCommitAuthor as getCICommitAuthor, getCommitMessage as getCICommitMessage, getPullRequestNumber, } from './ci-env.js'; @@ -151,8 +152,52 @@ export async function detectCommitMessage( let ciCommitMessage = getCICommitMessage(); if (ciCommitMessage) return ciCommitMessage; - // Fallback to regular git log - return await getCommitMessage(cwd); + // Fallback to git, reading the same commit reported as commit_sha + return await readCommit('%B', cwd); +} + +/** + * Read a formatted field from the detected build commit. + * + * Uses detectCommit() so CI checkouts (e.g. GitHub's synthetic PR merge + * commit) describe the PR head rather than HEAD. Falls back to HEAD when that + * commit is not in local history (shallow clones). + * + * @param {string} format - git log --format string + * @param {string} cwd - Working directory + * @returns {Promise} Formatted output or null + */ +async function readCommit(format, cwd = process.cwd()) { + let sha = await detectCommit(null, cwd); + let refs = /^[0-9a-f]{4,64}$/i.test(sha || '') ? [sha, 'HEAD'] : ['HEAD']; + + for (let ref of refs) { + try { + return await runGit(['log', '-1', `--format=${format}`, ref], cwd); + } catch { + // Commit not available locally, try the next ref + } + } + + return null; +} + +/** + * Detect commit author with environment variable support, falling back to git + * @param {string} cwd - Working directory + * @returns {Promise<{ name: string|null, email: string|null }>} + */ +export async function detectCommitAuthor(cwd = process.cwd()) { + let author = getCICommitAuthor(); + if (author.name && author.email) return author; + + let stdout = await readCommit('%an%x00%ae', cwd); + let [gitName, gitEmail] = stdout ? stdout.split('\0') : []; + + return { + name: author.name || gitName || null, + email: author.email || gitEmail || null, + }; } /** @@ -241,10 +286,10 @@ export async function generateBuildNameWithGit( ) { if (override) return override; - let branch = await getCurrentBranch(cwd); - let shortSha = await getCurrentCommitSha(cwd); + let branch = await detectBranch(null, cwd); + let shortSha = await detectCommit(null, cwd); - if (branch && shortSha) { + if (branch && branch !== 'unknown' && shortSha) { let shortCommit = shortSha.substring(0, 7); return `${branch}-${shortCommit}`; } diff --git a/tests/api/core.test.js b/tests/api/core.test.js index 572f7553..f544bcc1 100644 --- a/tests/api/core.test.js +++ b/tests/api/core.test.js @@ -347,6 +347,25 @@ describe('api/core', () => { }); describe('buildBuildPayload', () => { + it('includes commit author fields only when present', () => { + let base = { name: 'Build', branch: 'main', environment: 'test' }; + + let withAuthor = buildBuildPayload({ + ...base, + commit_author_name: 'Ada Lovelace', + commit_author_email: 'ada@example.com', + }); + let withoutAuthor = buildBuildPayload({ + ...base, + commit_author_name: null, + commit_author_email: undefined, + }); + + assert.strictEqual(withAuthor.commit_author_name, 'Ada Lovelace'); + assert.strictEqual(withAuthor.commit_author_email, 'ada@example.com'); + assert.deepStrictEqual(withoutAuthor, base); + }); + it('builds basic payload with name, branch, environment', () => { let result = buildBuildPayload({ name: 'My Build', diff --git a/tests/helpers/ci-env.js b/tests/helpers/ci-env.js new file mode 100644 index 00000000..ad3b9cb3 --- /dev/null +++ b/tests/helpers/ci-env.js @@ -0,0 +1,150 @@ +/** + * Shared CI environment isolation for tests that touch CI detection. + * + * Usage: + * let ciEnv = useCleanCIEnv(); + * process.env.GITHUB_EVENT_PATH = ciEnv.createEventFile({ ... }); + */ + +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach } from 'node:test'; +import { resetGitHubEventCache } from '../../src/utils/ci-env.js'; + +// CI variables read by src/utils/ci-env.js, cleared so host CI can't leak in +export let CI_ENV_VARS = [ + 'VIZZLY_BRANCH', + 'VIZZLY_COMMIT_SHA', + 'VIZZLY_COMMIT_MESSAGE', + 'VIZZLY_COMMIT_AUTHOR_NAME', + 'VIZZLY_COMMIT_AUTHOR_EMAIL', + 'VIZZLY_PR_NUMBER', + 'VIZZLY_PR_HEAD_SHA', + 'VIZZLY_PR_BASE_SHA', + 'VIZZLY_PR_HEAD_REF', + 'VIZZLY_PR_BASE_REF', + 'GITHUB_ACTIONS', + 'GITHUB_HEAD_REF', + 'GITHUB_REF_NAME', + 'GITHUB_SHA', + 'GITHUB_REF', + 'GITHUB_EVENT_NAME', + 'GITHUB_BASE_REF', + 'GITLAB_CI', + 'CI_COMMIT_REF_NAME', + 'CI_COMMIT_SHA', + 'CI_COMMIT_MESSAGE', + 'CI_COMMIT_AUTHOR', + 'CI_MERGE_REQUEST_ID', + 'CI_MERGE_REQUEST_SOURCE_BRANCH_NAME', + 'CI_MERGE_REQUEST_TARGET_BRANCH_NAME', + 'CI_MERGE_REQUEST_TARGET_BRANCH_SHA', + 'CIRCLECI', + 'CIRCLE_BRANCH', + 'CIRCLE_SHA1', + 'CIRCLE_PULL_REQUEST', + 'TRAVIS', + 'TRAVIS_BRANCH', + 'TRAVIS_COMMIT', + 'TRAVIS_COMMIT_MESSAGE', + 'TRAVIS_PULL_REQUEST', + 'TRAVIS_PULL_REQUEST_BRANCH', + 'BUILDKITE', + 'BUILDKITE_BRANCH', + 'BUILDKITE_COMMIT', + 'BUILDKITE_MESSAGE', + 'BUILDKITE_PULL_REQUEST', + 'BUILDKITE_PULL_REQUEST_BASE_BRANCH', + 'DRONE', + 'DRONE_BRANCH', + 'DRONE_COMMIT_SHA', + 'DRONE_COMMIT_MESSAGE', + 'DRONE_PULL_REQUEST', + 'DRONE_SOURCE_BRANCH', + 'DRONE_TARGET_BRANCH', + 'JENKINS_URL', + 'BRANCH_NAME', + 'GIT_BRANCH', + 'GIT_COMMIT', + 'ghprbPullId', + 'ghprbSourceBranch', + 'ghprbTargetBranch', + 'ghprbActualCommit', + 'BITBUCKET_BRANCH', + 'BITBUCKET_COMMIT', + 'BITBUCKET_BUILD_NUMBER', + 'WERCKER', + 'WERCKER_GIT_BRANCH', + 'WERCKER_GIT_COMMIT', + 'APPVEYOR', + 'APPVEYOR_REPO_BRANCH', + 'APPVEYOR_REPO_COMMIT', + 'APPVEYOR_REPO_COMMIT_MESSAGE', + 'APPVEYOR_PULL_REQUEST_NUMBER', + 'APPVEYOR_PULL_REQUEST_HEAD_REPO_BRANCH', + 'TF_BUILD', + 'AZURE_HTTP_USER_AGENT', + 'BUILD_SOURCEBRANCH', + 'BUILD_SOURCEVERSION', + 'SYSTEM_PULLREQUEST_PULLREQUESTID', + 'SYSTEM_PULLREQUEST_SOURCEBRANCH', + 'SYSTEM_PULLREQUEST_TARGETBRANCH', + 'CODEBUILD_BUILD_ID', + 'CODEBUILD_WEBHOOK_HEAD_REF', + 'CODEBUILD_RESOLVED_SOURCE_VERSION', + 'SEMAPHORE', + 'SEMAPHORE_GIT_BRANCH', + 'SEMAPHORE_GIT_SHA', + 'HEROKU_TEST_RUN_ID', + 'HEROKU_TEST_RUN_COMMIT_VERSION', + 'COMMIT_SHA', + 'HEAD_COMMIT', + 'SHA', + 'COMMIT_MESSAGE', + 'GITHUB_EVENT_PATH', +]; + +/** + * Clear CI env vars and the GitHub event cache around each test + * @returns {{ createEventFile: (payload: Object|string) => string }} + */ +export function useCleanCIEnv() { + let originalEnv; + let tempDirs = []; + + beforeEach(() => { + originalEnv = { ...process.env }; + tempDirs = []; + for (let name of CI_ENV_VARS) { + delete process.env[name]; + } + resetGitHubEventCache(); + }); + + afterEach(() => { + for (let dir of tempDirs) { + rmSync(dir, { recursive: true, force: true }); + } + process.env = originalEnv; + resetGitHubEventCache(); + }); + + return { + /** + * Write a GitHub Actions event payload to a temp file + * @param {Object|string} payload - Event payload (strings written as-is) + * @returns {string} Path to the event file + */ + createEventFile(payload) { + let dir = mkdtempSync(join(tmpdir(), 'vizzly-event-')); + tempDirs.push(dir); + let eventPath = join(dir, 'event.json'); + writeFileSync( + eventPath, + typeof payload === 'string' ? payload : JSON.stringify(payload) + ); + return eventPath; + }, + }; +} diff --git a/tests/test-runner/core.test.js b/tests/test-runner/core.test.js index 5eb81ea1..36f25579 100644 --- a/tests/test-runner/core.test.js +++ b/tests/test-runner/core.test.js @@ -108,6 +108,8 @@ describe('test-runner/core', () => { environment: 'staging', commit: 'abc123', message: 'Test commit', + commitAuthorName: 'Ada Lovelace', + commitAuthorEmail: 'ada@example.com', pullRequestNumber: 42, parallelId: 'parallel-1', }; @@ -120,6 +122,8 @@ describe('test-runner/core', () => { environment: 'staging', commit_sha: 'abc123', commit_message: 'Test commit', + commit_author_name: 'Ada Lovelace', + commit_author_email: 'ada@example.com', github_pull_request_number: 42, parallel_id: 'parallel-1', }); diff --git a/tests/uploader/core.test.js b/tests/uploader/core.test.js index 541f8ddb..c451615f 100644 --- a/tests/uploader/core.test.js +++ b/tests/uploader/core.test.js @@ -175,6 +175,8 @@ describe('uploader/core', () => { branch: 'feature/test', commit: 'abc123', message: 'Test commit', + commitAuthorName: 'Ada Lovelace', + commitAuthorEmail: 'ada@example.com', environment: 'staging', threshold: 0.05, minClusterSize: 4, @@ -193,6 +195,8 @@ describe('uploader/core', () => { branch: 'feature/test', commit_sha: 'abc123', commit_message: 'Test commit', + commit_author_name: 'Ada Lovelace', + commit_author_email: 'ada@example.com', environment: 'staging', threshold: 0.05, metadata: { diff --git a/tests/uploader/index.test.js b/tests/uploader/index.test.js index 4abddb17..7745963f 100644 --- a/tests/uploader/index.test.js +++ b/tests/uploader/index.test.js @@ -127,6 +127,8 @@ describe('uploader/createUploader', () => { branch: 'feature/reports', commit_sha: undefined, commit_message: undefined, + commit_author_name: undefined, + commit_author_email: undefined, environment: 'staging', threshold: undefined, metadata: { diff --git a/tests/utils/ci-env.test.js b/tests/utils/ci-env.test.js index 85f8b95f..5abd3dd0 100644 --- a/tests/utils/ci-env.test.js +++ b/tests/utils/ci-env.test.js @@ -1,12 +1,11 @@ import assert from 'node:assert'; -import { mkdtempSync, unlinkSync, writeFileSync } from 'node:fs'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; -import { afterEach, beforeEach, describe, it } from 'node:test'; +import { unlinkSync } from 'node:fs'; +import { describe, it } from 'node:test'; import { getBranch, getCIProvider, getCommit, + getCommitAuthor, getCommitMessage, getGitHubEvent, getPullRequestBaseRef, @@ -17,131 +16,10 @@ import { isPullRequest, resetGitHubEventCache, } from '../../src/utils/ci-env.js'; +import { useCleanCIEnv } from '../helpers/ci-env.js'; describe('utils/ci-env', () => { - let originalEnv; - let tempFilesToCleanup = []; - - beforeEach(() => { - originalEnv = { ...process.env }; - tempFilesToCleanup = []; - // Clear all CI-related env vars - let ciVars = [ - 'VIZZLY_BRANCH', - 'VIZZLY_COMMIT_SHA', - 'VIZZLY_COMMIT_MESSAGE', - 'VIZZLY_PR_NUMBER', - 'VIZZLY_PR_HEAD_SHA', - 'VIZZLY_PR_BASE_SHA', - 'VIZZLY_PR_HEAD_REF', - 'VIZZLY_PR_BASE_REF', - 'GITHUB_ACTIONS', - 'GITHUB_HEAD_REF', - 'GITHUB_REF_NAME', - 'GITHUB_SHA', - 'GITHUB_REF', - 'GITHUB_EVENT_NAME', - 'GITHUB_BASE_REF', - 'GITLAB_CI', - 'CI_COMMIT_REF_NAME', - 'CI_COMMIT_SHA', - 'CI_COMMIT_MESSAGE', - 'CI_MERGE_REQUEST_ID', - 'CI_MERGE_REQUEST_SOURCE_BRANCH_NAME', - 'CI_MERGE_REQUEST_TARGET_BRANCH_NAME', - 'CI_MERGE_REQUEST_TARGET_BRANCH_SHA', - 'CIRCLECI', - 'CIRCLE_BRANCH', - 'CIRCLE_SHA1', - 'CIRCLE_PULL_REQUEST', - 'TRAVIS', - 'TRAVIS_BRANCH', - 'TRAVIS_COMMIT', - 'TRAVIS_COMMIT_MESSAGE', - 'TRAVIS_PULL_REQUEST', - 'TRAVIS_PULL_REQUEST_BRANCH', - 'BUILDKITE', - 'BUILDKITE_BRANCH', - 'BUILDKITE_COMMIT', - 'BUILDKITE_MESSAGE', - 'BUILDKITE_PULL_REQUEST', - 'BUILDKITE_PULL_REQUEST_BASE_BRANCH', - 'DRONE', - 'DRONE_BRANCH', - 'DRONE_COMMIT_SHA', - 'DRONE_COMMIT_MESSAGE', - 'DRONE_PULL_REQUEST', - 'DRONE_SOURCE_BRANCH', - 'DRONE_TARGET_BRANCH', - 'JENKINS_URL', - 'BRANCH_NAME', - 'GIT_BRANCH', - 'GIT_COMMIT', - 'ghprbPullId', - 'ghprbSourceBranch', - 'ghprbTargetBranch', - 'ghprbActualCommit', - 'BITBUCKET_BRANCH', - 'BITBUCKET_COMMIT', - 'BITBUCKET_BUILD_NUMBER', - 'WERCKER', - 'WERCKER_GIT_BRANCH', - 'WERCKER_GIT_COMMIT', - 'APPVEYOR', - 'APPVEYOR_REPO_BRANCH', - 'APPVEYOR_REPO_COMMIT', - 'APPVEYOR_REPO_COMMIT_MESSAGE', - 'APPVEYOR_PULL_REQUEST_NUMBER', - 'APPVEYOR_PULL_REQUEST_HEAD_REPO_BRANCH', - 'TF_BUILD', - 'AZURE_HTTP_USER_AGENT', - 'BUILD_SOURCEBRANCH', - 'BUILD_SOURCEVERSION', - 'SYSTEM_PULLREQUEST_PULLREQUESTID', - 'SYSTEM_PULLREQUEST_SOURCEBRANCH', - 'SYSTEM_PULLREQUEST_TARGETBRANCH', - 'CODEBUILD_BUILD_ID', - 'CODEBUILD_WEBHOOK_HEAD_REF', - 'CODEBUILD_RESOLVED_SOURCE_VERSION', - 'SEMAPHORE', - 'SEMAPHORE_GIT_BRANCH', - 'SEMAPHORE_GIT_SHA', - 'HEROKU_TEST_RUN_ID', - 'HEROKU_TEST_RUN_COMMIT_VERSION', - 'COMMIT_SHA', - 'HEAD_COMMIT', - 'SHA', - 'COMMIT_MESSAGE', - 'GITHUB_EVENT_PATH', - ]; - for (let v of ciVars) { - delete process.env[v]; - } - // Reset the GitHub event cache between tests - resetGitHubEventCache(); - }); - - afterEach(() => { - for (let file of tempFilesToCleanup) { - try { - unlinkSync(file); - } catch { - // File was already cleaned up or does not exist. - } - } - process.env = originalEnv; - }); - - function createTempEventFile(payload) { - let tempDir = mkdtempSync(join(tmpdir(), 'vizzly-test-')); - let eventPath = join(tempDir, 'event.json'); - writeFileSync( - eventPath, - typeof payload === 'string' ? payload : JSON.stringify(payload) - ); - tempFilesToCleanup.push(eventPath); - return eventPath; - } + let ciEnv = useCleanCIEnv(); describe('getBranch', () => { it('returns null when no CI env vars set', () => { @@ -212,7 +90,7 @@ describe('utils/ci-env', () => { }); it('reads the PR head SHA from a GitHub Actions event file', () => { - let eventPath = createTempEventFile({ + let eventPath = ciEnv.createEventFile({ pull_request: { head: { sha: 'pr-head-sha-abc123' }, base: { sha: 'base-sha-def456' }, @@ -239,7 +117,7 @@ describe('utils/ci-env', () => { }); it('falls back to GITHUB_SHA when the event is not a pull request', () => { - let eventPath = createTempEventFile({ ref: 'refs/heads/main' }); + let eventPath = ciEnv.createEventFile({ ref: 'refs/heads/main' }); process.env.GITHUB_ACTIONS = 'true'; process.env.GITHUB_EVENT_PATH = eventPath; @@ -274,6 +152,70 @@ describe('utils/ci-env', () => { assert.strictEqual(getCommitMessage(), 'fix: bug'); }); + + it('uses the PR title for GitHub Actions pull_request events', () => { + process.env.GITHUB_ACTIONS = 'true'; + process.env.GITHUB_EVENT_PATH = ciEnv.createEventFile({ + pull_request: { title: 'Add dark mode', head: { sha: 'abc' } }, + }); + + assert.strictEqual(getCommitMessage(), 'Add dark mode'); + }); + + it('prefers VIZZLY_COMMIT_MESSAGE over the PR title', () => { + process.env.VIZZLY_COMMIT_MESSAGE = 'vizzly message'; + process.env.GITHUB_ACTIONS = 'true'; + process.env.GITHUB_EVENT_PATH = ciEnv.createEventFile({ + pull_request: { title: 'Add dark mode' }, + }); + + assert.strictEqual(getCommitMessage(), 'vizzly message'); + }); + + it('returns null for GitHub Actions push events', () => { + process.env.GITHUB_ACTIONS = 'true'; + process.env.GITHUB_EVENT_PATH = ciEnv.createEventFile({ + ref: 'refs/heads/main', + }); + + assert.strictEqual(getCommitMessage(), null); + }); + }); + + describe('getCommitAuthor', () => { + it('returns nulls when no CI env vars set', () => { + assert.deepStrictEqual(getCommitAuthor(), { name: null, email: null }); + }); + + it('reads VIZZLY_COMMIT_AUTHOR_NAME and VIZZLY_COMMIT_AUTHOR_EMAIL', () => { + process.env.VIZZLY_COMMIT_AUTHOR_NAME = 'Ada Lovelace'; + process.env.VIZZLY_COMMIT_AUTHOR_EMAIL = 'ada@example.com'; + process.env.CI_COMMIT_AUTHOR = 'Someone Else '; + + assert.deepStrictEqual(getCommitAuthor(), { + name: 'Ada Lovelace', + email: 'ada@example.com', + }); + }); + + it('parses GitLab CI_COMMIT_AUTHOR', () => { + process.env.CI_COMMIT_AUTHOR = 'Grace Hopper '; + + assert.deepStrictEqual(getCommitAuthor(), { + name: 'Grace Hopper', + email: 'grace@example.com', + }); + }); + + it('lets a single override win per field', () => { + process.env.VIZZLY_COMMIT_AUTHOR_EMAIL = 'override@example.com'; + process.env.CI_COMMIT_AUTHOR = 'Grace Hopper '; + + assert.deepStrictEqual(getCommitAuthor(), { + name: 'Grace Hopper', + email: 'override@example.com', + }); + }); }); describe('getPullRequestNumber', () => { @@ -403,7 +345,7 @@ describe('utils/ci-env', () => { }); it('reads the PR head SHA from a GitHub Actions event file', () => { - let eventPath = createTempEventFile({ + let eventPath = ciEnv.createEventFile({ pull_request: { head: { sha: 'pr-head-sha-from-event' }, base: { sha: 'base-sha' }, @@ -437,7 +379,7 @@ describe('utils/ci-env', () => { }); it('reads the PR base SHA from a GitHub Actions event file', () => { - let eventPath = createTempEventFile({ + let eventPath = ciEnv.createEventFile({ pull_request: { head: { sha: 'head-sha' }, base: { sha: 'base-sha-from-event' }, @@ -459,7 +401,7 @@ describe('utils/ci-env', () => { }); it('parses and caches the event file', () => { - let eventPath = createTempEventFile({ action: 'opened', number: 42 }); + let eventPath = ciEnv.createEventFile({ action: 'opened', number: 42 }); process.env.GITHUB_EVENT_PATH = eventPath; resetGitHubEventCache(); @@ -468,15 +410,12 @@ describe('utils/ci-env', () => { assert.deepStrictEqual(event, { action: 'opened', number: 42 }); unlinkSync(eventPath); - tempFilesToCleanup = tempFilesToCleanup.filter( - file => file !== eventPath - ); assert.deepStrictEqual(getGitHubEvent(), event); }); it('returns an empty object for invalid JSON', () => { - let eventPath = createTempEventFile('not valid json {{{'); + let eventPath = ciEnv.createEventFile('not valid json {{{'); process.env.GITHUB_EVENT_PATH = eventPath; resetGitHubEventCache(); diff --git a/tests/utils/git.test.js b/tests/utils/git.test.js index 6f0b7c80..afb93e2f 100644 --- a/tests/utils/git.test.js +++ b/tests/utils/git.test.js @@ -8,6 +8,7 @@ import { promisify } from 'node:util'; import { detectBranch, detectCommit, + detectCommitAuthor, detectCommitMessage, detectPullRequestNumber, generateBuildName, @@ -20,6 +21,7 @@ import { getGitStatus, isGitRepository, } from '../../src/utils/git.js'; +import { useCleanCIEnv } from '../helpers/ci-env.js'; let execFileAsync = promisify(execFile); @@ -27,6 +29,38 @@ async function runGit(cwd, args) { await execFileAsync('git', args, { cwd }); } +async function gitOutput(cwd, args) { + let { stdout } = await execFileAsync('git', args, { cwd }); + return stdout.trim(); +} + +// Recreate GitHub's refs/pull/N/merge checkout: a merge commit of the PR +// head into base, with GitHub's synthetic "Merge into " message +async function createGitHubMergeCheckout(directory) { + let base = await gitOutput(directory, ['rev-parse', 'HEAD']); + await runGit(directory, ['checkout', '-q', '-b', 'feature']); + await writeFile(join(directory, 'feature.txt'), 'feature\n'); + await runGit(directory, ['add', 'feature.txt']); + await runGit(directory, [ + 'commit', + '-q', + '--author=PR Author ', + '-m', + 'Add feature work', + ]); + let head = await gitOutput(directory, ['rev-parse', 'HEAD']); + await runGit(directory, ['checkout', '-q', '--detach', base]); + await runGit(directory, [ + 'merge', + '-q', + '--no-ff', + '-m', + `Merge ${head} into ${base}`, + head, + ]); + return { base, head }; +} + async function withGitRepo(testFn) { let directory = await mkdtemp(join(tmpdir(), 'vizzly-git-')); @@ -45,6 +79,15 @@ async function withGitRepo(testFn) { } describe('utils/git', () => { + let ciEnv = useCleanCIEnv(); + + function usePullRequestEvent(pullRequest) { + process.env.GITHUB_ACTIONS = 'true'; + process.env.GITHUB_EVENT_PATH = ciEnv.createEventFile({ + pull_request: pullRequest, + }); + } + describe('generateBuildName', () => { it('generates build name with timestamp', () => { let name = generateBuildName(); @@ -205,6 +248,48 @@ describe('utils/git', () => { }); describe('detectCommitMessage', () => { + it('uses the PR title from the GitHub event payload', async () => { + await withGitRepo(async directory => { + usePullRequestEvent({ title: 'Add dark mode', head: { sha: 'abc' } }); + + let message = await detectCommitMessage(null, directory); + + assert.strictEqual(message, 'Add dark mode'); + }); + }); + + it('reads the PR head commit on a GitHub merge checkout', async () => { + await withGitRepo(async directory => { + let { head } = await createGitHubMergeCheckout(directory); + usePullRequestEvent({ head: { sha: head } }); + + let message = await detectCommitMessage(null, directory); + let author = await detectCommitAuthor(directory); + + assert.strictEqual(message, 'Add feature work'); + assert.deepStrictEqual(author, { + name: 'PR Author', + email: 'pr@example.com', + }); + }); + }); + + it('falls back to HEAD when the PR head is not in local history', async () => { + await withGitRepo(async directory => { + let { base, head } = await createGitHubMergeCheckout(directory); + usePullRequestEvent({ head: { sha: 'f'.repeat(40) } }); + + let message = await detectCommitMessage(null, directory); + let author = await detectCommitAuthor(directory); + + assert.strictEqual(message, `Merge ${head} into ${base}`); + assert.deepStrictEqual(author, { + name: 'Vizzly Test', + email: 'test@example.com', + }); + }); + }); + it('returns override if provided', async () => { let message = await detectCommitMessage('Custom message'); @@ -220,7 +305,62 @@ describe('utils/git', () => { }); }); + describe('detectCommitAuthor', () => { + it('reads the author from git', async () => { + await withGitRepo(async directory => { + let author = await detectCommitAuthor(directory); + + assert.deepStrictEqual(author, { + name: 'Vizzly Test', + email: 'test@example.com', + }); + }); + }); + + it('prefers VIZZLY_COMMIT_AUTHOR_* overrides', async () => { + await withGitRepo(async directory => { + process.env.VIZZLY_COMMIT_AUTHOR_NAME = 'Ada Lovelace'; + + let author = await detectCommitAuthor(directory); + + assert.deepStrictEqual(author, { + name: 'Ada Lovelace', + email: 'test@example.com', + }); + }); + }); + + it('returns nulls outside a git repository', async () => { + let author = await detectCommitAuthor('/non-existent-path-12345'); + + assert.deepStrictEqual(author, { name: null, email: null }); + }); + }); + describe('generateBuildNameWithGit', () => { + it('names GitHub PR builds after the head branch and head commit', async () => { + await withGitRepo(async directory => { + let { head } = await createGitHubMergeCheckout(directory); + usePullRequestEvent({ head: { sha: head } }); + process.env.GITHUB_HEAD_REF = 'feature/dark-mode'; + + let name = await generateBuildNameWithGit(null, directory); + + assert.strictEqual(name, `feature/dark-mode-${head.slice(0, 7)}`); + }); + }); + + it('uses the local branch and commit outside CI', async () => { + await withGitRepo(async directory => { + await runGit(directory, ['checkout', '-q', '-b', 'local-work']); + let sha = await gitOutput(directory, ['rev-parse', 'HEAD']); + + let name = await generateBuildNameWithGit(null, directory); + + assert.strictEqual(name, `local-work-${sha.slice(0, 7)}`); + }); + }); + it('returns override if provided', async () => { let name = await generateBuildNameWithGit('Custom Build');