mirror of
https://github.com/stablyai/orca.git
synced 2026-11-02 10:08:13 +00:00
Correct GitLab MR checks for failed and manual pipelines (#34176)
* Correct GitLab MR check status for failed and manual pipelines Use the same single-MR read for linked and branch-discovered reviews so a list summary cannot hide a failed pipeline. Preserve lookup failures for strict callers and keep detail reads within the existing branch cache. Set mergeable manual pipelines to neutral without claiming an unrun stage passed; keep blocking manual pipelines pending. Co-authored-by: Son of Odin <2700975-son-of-odin@users.noreply.gitlab.com> Co-authored-by: Emily <hadikcze@gmail.com> * test: cover GitLab MR check status consumers * test: type GitLab MR fixtures from mapper input * chore: document GitLab MR boundary assertion exceptions * test: share GitLab MR producer and consumer fixtures --------- Co-authored-by: Son of Odin <2700975-son-of-odin@users.noreply.gitlab.com> Co-authored-by: Emily <hadikcze@gmail.com>
This commit is contained in:
co-authored by
Son of Odin
Emily
parent
41755b5b24
commit
6ecd992f44
@@ -1,5 +1,12 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type * as GlUtils from './gl-utils'
|
||||
import { derivePipelineStatus, mapMRInfo } from './mappers'
|
||||
import { gitlabMrCheckCases } from '../../shared/gitlab-mr-checks.test-fixture'
|
||||
import { mapGitLabReview } from '../source-control/forge-review-mappers'
|
||||
import {
|
||||
__resetHostedReviewBranchCacheForTests,
|
||||
withHostedReviewBranchCache
|
||||
} from '../source-control/hosted-review-branch-cache'
|
||||
|
||||
const {
|
||||
glabExecFileAsyncMock,
|
||||
@@ -48,7 +55,20 @@ import {
|
||||
} from './client'
|
||||
import { resetGitLabMrMocks } from './client-mr-test-harness'
|
||||
|
||||
type MergeRequestDetail = Parameters<typeof mapMRInfo>[0]
|
||||
|
||||
function mockBranchLookup(mr: MergeRequestDetail): void {
|
||||
glabExecFileAsyncMock
|
||||
.mockResolvedValueOnce({ stdout: JSON.stringify([mr]) })
|
||||
.mockResolvedValueOnce({ stdout: JSON.stringify(mr) })
|
||||
}
|
||||
|
||||
describe('gitlab client — MR operations', () => {
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
__resetHostedReviewBranchCacheForTests()
|
||||
})
|
||||
|
||||
beforeEach(() => {
|
||||
resetGitLabMrMocks({
|
||||
glabExecFileAsyncMock,
|
||||
@@ -73,6 +93,12 @@ describe('gitlab client — MR operations', () => {
|
||||
)
|
||||
})
|
||||
|
||||
it.each(gitlabMrCheckCases)('maps $name to the shared consumer review', ({ raw, review }) => {
|
||||
expect(mapGitLabReview(mapMRInfo(raw, derivePipelineStatus(raw.head_pipeline)))).toStrictEqual(
|
||||
review
|
||||
)
|
||||
})
|
||||
|
||||
describe('getMergeRequest', () => {
|
||||
it('fetches the MR with rolled-up pipeline status', async () => {
|
||||
getProjectRefMock.mockResolvedValueOnce({ host: 'gitlab.com', path: 'g/p' })
|
||||
@@ -137,18 +163,36 @@ describe('gitlab client — MR operations', () => {
|
||||
})
|
||||
|
||||
describe('getMergeRequestForBranch', () => {
|
||||
it('settles mergeable manual pipelines consistently for by-number, linked and discovered MRs', async () => {
|
||||
getProjectRefMock.mockResolvedValue({ host: 'gitlab.com', path: 'g/p' })
|
||||
const detail = {
|
||||
iid: 7,
|
||||
title: 'Manual release',
|
||||
state: 'opened',
|
||||
head_pipeline: { status: 'manual' },
|
||||
detailed_merge_status: 'mergeable'
|
||||
}
|
||||
glabExecFileAsyncMock.mockImplementation(async (args: string[]) => ({
|
||||
stdout: JSON.stringify(args[1].includes('source_branch=') ? [{ iid: 7 }] : detail)
|
||||
}))
|
||||
const reviews = await Promise.all([
|
||||
getMergeRequest('/repo', 7),
|
||||
getMergeRequestForBranch('/repo', 'feature/manual', 7),
|
||||
getMergeRequestForBranch('/repo', 'feature/manual')
|
||||
])
|
||||
expect(reviews.every((mr) => mr?.pipelineStatus === 'neutral')).toBe(true)
|
||||
expect(reviews.every((mr) => mr?.mergeable === 'MERGEABLE')).toBe(true)
|
||||
expect(glabExecFileAsyncMock).toHaveBeenCalledTimes(4)
|
||||
})
|
||||
|
||||
it('finds the most recently updated MR for a branch across states', async () => {
|
||||
getProjectRefMock.mockResolvedValueOnce({ host: 'gitlab.com', path: 'g/p' })
|
||||
glabExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([
|
||||
{
|
||||
iid: 7,
|
||||
title: 'WIP',
|
||||
state: 'merged',
|
||||
sha: 'abc',
|
||||
head_pipeline: { status: 'success' }
|
||||
}
|
||||
])
|
||||
mockBranchLookup({
|
||||
iid: 7,
|
||||
title: 'WIP',
|
||||
state: 'merged',
|
||||
sha: 'abc',
|
||||
head_pipeline: { status: 'success' }
|
||||
})
|
||||
|
||||
const mr = await getMergeRequestForBranch('/repo', 'feature/foo')
|
||||
@@ -176,16 +220,12 @@ describe('gitlab client — MR operations', () => {
|
||||
|
||||
it('uses legacy pipeline payloads when branch MR lists omit head_pipeline', async () => {
|
||||
getProjectRefMock.mockResolvedValueOnce({ host: 'gitlab.com', path: 'g/p' })
|
||||
glabExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([
|
||||
{
|
||||
iid: 8,
|
||||
title: 'Legacy pipeline branch',
|
||||
state: 'opened',
|
||||
sha: 'def',
|
||||
pipeline: { status: 'failed' }
|
||||
}
|
||||
])
|
||||
mockBranchLookup({
|
||||
iid: 8,
|
||||
title: 'Legacy pipeline branch',
|
||||
state: 'opened',
|
||||
sha: 'def',
|
||||
pipeline: { status: 'failed' }
|
||||
})
|
||||
|
||||
const mr = await getMergeRequestForBranch('/repo', 'feature/legacy-pipeline')
|
||||
@@ -193,6 +233,95 @@ describe('gitlab client — MR operations', () => {
|
||||
expect(mr?.pipelineStatus).toBe('failure')
|
||||
})
|
||||
|
||||
it.each([{}, { head_pipeline: null }, { head_pipeline: { status: 'success' } }])(
|
||||
'reads the failed head pipeline from detail instead of the branch list: %j',
|
||||
async (pipelineFields) => {
|
||||
const detail = {
|
||||
iid: 5049,
|
||||
title: 'Pipeline regression',
|
||||
state: 'opened',
|
||||
sha: 'abc',
|
||||
head_pipeline: { status: 'failed' }
|
||||
}
|
||||
getProjectRefMock.mockResolvedValue({ host: 'gitlab.com', path: 'g/p' })
|
||||
glabExecFileAsyncMock
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([{ ...detail, head_pipeline: undefined, ...pipelineFields }])
|
||||
})
|
||||
.mockResolvedValueOnce({ stdout: JSON.stringify(detail) })
|
||||
.mockResolvedValueOnce({ stdout: JSON.stringify(detail) })
|
||||
|
||||
const discovered = await getMergeRequestForBranch('/repo', 'feature/pipeline')
|
||||
const linked = await getMergeRequestForBranch('/repo', 'feature/pipeline', 5049)
|
||||
expect(discovered).toEqual(linked)
|
||||
expect(discovered).toMatchObject({ number: 5049, pipelineStatus: 'failure' })
|
||||
expect(glabExecFileAsyncMock).toHaveBeenCalledTimes(3)
|
||||
expect(glabExecFileAsyncMock.mock.calls[1]).toEqual(glabExecFileAsyncMock.mock.calls[2])
|
||||
}
|
||||
)
|
||||
|
||||
it('preserves detail failures for strict callers instead of returning a neutral list row', async () => {
|
||||
getProjectRefMock.mockResolvedValue({ host: 'gitlab.com', path: 'g/p' })
|
||||
for (const lookup of [getMergeRequestForBranch, getMergeRequestForBranchOrThrow]) {
|
||||
glabExecFileAsyncMock
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([{ iid: 6, title: 'Flaky', state: 'opened' }])
|
||||
})
|
||||
.mockRejectedValueOnce(new Error('glab: 502'))
|
||||
const result = lookup('/repo', 'flaky')
|
||||
await (lookup === getMergeRequestForBranchOrThrow
|
||||
? expect(result).rejects.toThrow('glab: 502')
|
||||
: expect(result).resolves.toBeNull())
|
||||
}
|
||||
glabExecFileAsyncMock.mockRejectedValueOnce(new Error('glab: 502'))
|
||||
await expect(getMergeRequestForBranchOrThrow('/repo', 'flaky', 6)).rejects.toThrow(
|
||||
'glab: 502'
|
||||
)
|
||||
expect(glabExecFileAsyncMock.mock.calls[4]).toEqual(glabExecFileAsyncMock.mock.calls[3])
|
||||
expect(glabExecFileAsyncMock).toHaveBeenCalledTimes(5)
|
||||
expect(releaseMock).toHaveBeenCalledTimes(3)
|
||||
})
|
||||
|
||||
it.each([10, 100])(
|
||||
'caches list and detail together while polling %i branch rows',
|
||||
async (rows) => {
|
||||
vi.useFakeTimers()
|
||||
getProjectRefMock.mockResolvedValue({ host: 'gitlab.com', path: 'g/p' })
|
||||
const detail = {
|
||||
iid: 7,
|
||||
title: 'CI failure',
|
||||
state: 'opened',
|
||||
head_pipeline: { status: 'failed' }
|
||||
}
|
||||
glabExecFileAsyncMock.mockImplementation(async (args: string[]) => ({
|
||||
stdout: JSON.stringify(args[1].includes('source_branch=') ? [{ iid: 7 }] : detail)
|
||||
}))
|
||||
const poll = () =>
|
||||
Promise.all(
|
||||
Array.from({ length: rows }, (_, row) => {
|
||||
const branch = `feature/${row}`
|
||||
return withHostedReviewBranchCache(
|
||||
{ repoPath: '/repo', executionHostId: 'local', branch },
|
||||
{ headOid: null },
|
||||
async () => {
|
||||
const mr = await getMergeRequestForBranchOrThrow('/repo', branch)
|
||||
return mr ? mapGitLabReview(mr) : null
|
||||
}
|
||||
)
|
||||
})
|
||||
)
|
||||
const results = await Promise.all([poll(), poll()])
|
||||
expect(results[0]).toHaveLength(rows)
|
||||
expect(results[0].every((review) => review?.status === 'failure')).toBe(true)
|
||||
await vi.advanceTimersByTimeAsync(59_999)
|
||||
await poll()
|
||||
expect(glabExecFileAsyncMock).toHaveBeenCalledTimes(2 * rows)
|
||||
await vi.advanceTimersByTimeAsync(1)
|
||||
await poll()
|
||||
expect(glabExecFileAsyncMock).toHaveBeenCalledTimes(4 * rows)
|
||||
}
|
||||
)
|
||||
|
||||
it('strips refs/heads/ prefix from the branch arg', async () => {
|
||||
getProjectRefMock.mockResolvedValueOnce({ host: 'gitlab.com', path: 'g/p' })
|
||||
glabExecFileAsyncMock.mockResolvedValueOnce({ stdout: '[]' })
|
||||
@@ -299,16 +428,12 @@ describe('gitlab client — MR operations', () => {
|
||||
|
||||
it('routes local WSL merge-request branch lookup through the selected distro', async () => {
|
||||
getProjectRefMock.mockResolvedValueOnce({ host: 'gitlab.com', path: 'g/p' })
|
||||
glabExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([
|
||||
{
|
||||
iid: 12,
|
||||
title: 'WSL branch',
|
||||
state: 'opened',
|
||||
sha: 'abc',
|
||||
head_pipeline: { status: 'success' }
|
||||
}
|
||||
])
|
||||
mockBranchLookup({
|
||||
iid: 12,
|
||||
title: 'WSL branch',
|
||||
state: 'opened',
|
||||
sha: 'abc',
|
||||
head_pipeline: { status: 'success' }
|
||||
})
|
||||
|
||||
const mr = await getMergeRequestForBranch('/repo', 'feature/wsl', null, null, {
|
||||
@@ -327,16 +452,12 @@ describe('gitlab client — MR operations', () => {
|
||||
|
||||
it('hides a stale closed MR whose source branch is the repo default branch (#9171)', async () => {
|
||||
getProjectRefMock.mockResolvedValueOnce({ host: 'gitlab.com', path: 'g/p' })
|
||||
glabExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([
|
||||
{
|
||||
iid: 7,
|
||||
title: 'Accidental MR from main',
|
||||
state: 'closed',
|
||||
sha: 'stale-main-oid',
|
||||
head_pipeline: { status: 'success' }
|
||||
}
|
||||
])
|
||||
mockBranchLookup({
|
||||
iid: 7,
|
||||
title: 'Accidental MR from main',
|
||||
state: 'closed',
|
||||
sha: 'stale-main-oid',
|
||||
head_pipeline: { status: 'success' }
|
||||
})
|
||||
|
||||
await expect(getMergeRequestForBranch('/repo', 'main')).resolves.toBeNull()
|
||||
@@ -344,16 +465,12 @@ describe('gitlab client — MR operations', () => {
|
||||
|
||||
it('hides a stuck-locked MR whose source branch is the repo default branch (#9171)', async () => {
|
||||
getProjectRefMock.mockResolvedValueOnce({ host: 'gitlab.com', path: 'g/p' })
|
||||
glabExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([
|
||||
{
|
||||
iid: 9,
|
||||
title: 'Wedged mid-merge MR from main',
|
||||
state: 'locked',
|
||||
sha: 'locked-main-oid',
|
||||
head_pipeline: { status: 'success' }
|
||||
}
|
||||
])
|
||||
mockBranchLookup({
|
||||
iid: 9,
|
||||
title: 'Wedged mid-merge MR from main',
|
||||
state: 'locked',
|
||||
sha: 'locked-main-oid',
|
||||
head_pipeline: { status: 'success' }
|
||||
})
|
||||
|
||||
await expect(getMergeRequestForBranch('/repo', 'main')).resolves.toBeNull()
|
||||
@@ -361,16 +478,12 @@ describe('gitlab client — MR operations', () => {
|
||||
|
||||
it('keeps an open MR whose source branch is the repo default branch', async () => {
|
||||
getProjectRefMock.mockResolvedValueOnce({ host: 'gitlab.com', path: 'g/p' })
|
||||
glabExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([
|
||||
{
|
||||
iid: 8,
|
||||
title: 'main → release',
|
||||
state: 'opened',
|
||||
sha: 'abc',
|
||||
head_pipeline: { status: 'success' }
|
||||
}
|
||||
])
|
||||
mockBranchLookup({
|
||||
iid: 8,
|
||||
title: 'main → release',
|
||||
state: 'opened',
|
||||
sha: 'abc',
|
||||
head_pipeline: { status: 'success' }
|
||||
})
|
||||
|
||||
const mr = await getMergeRequestForBranch('/repo', 'main')
|
||||
@@ -380,16 +493,12 @@ describe('gitlab client — MR operations', () => {
|
||||
|
||||
it('keeps a closed MR on a feature branch (behavior preserved)', async () => {
|
||||
getProjectRefMock.mockResolvedValueOnce({ host: 'gitlab.com', path: 'g/p' })
|
||||
glabExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([
|
||||
{
|
||||
iid: 9,
|
||||
title: 'Rejected work',
|
||||
state: 'closed',
|
||||
sha: 'def',
|
||||
head_pipeline: { status: 'failed' }
|
||||
}
|
||||
])
|
||||
mockBranchLookup({
|
||||
iid: 9,
|
||||
title: 'Rejected work',
|
||||
state: 'closed',
|
||||
sha: 'def',
|
||||
head_pipeline: { status: 'failed' }
|
||||
})
|
||||
|
||||
const mr = await getMergeRequestForBranch('/repo', 'feature/rejected')
|
||||
|
||||
@@ -130,6 +130,32 @@ describe('mapGitLabIssueInfo', () => {
|
||||
})
|
||||
|
||||
describe('mapMRInfo', () => {
|
||||
it('settles mergeable manual pipelines without claiming they passed, while preserving blockers', () => {
|
||||
for (const pipelineField of ['head_pipeline', 'pipeline']) {
|
||||
for (const [status, detailed_merge_status, expected] of [
|
||||
['manual', 'mergeable', 'neutral'],
|
||||
['manual', 'ci_must_pass', 'pending'],
|
||||
['manual', 'merge_request_blocked', 'pending'],
|
||||
['manual', 'draft_status', 'pending'],
|
||||
['manual', undefined, 'pending'],
|
||||
['running', 'mergeable', 'pending'],
|
||||
['failed', 'mergeable', 'failure']
|
||||
] as const) {
|
||||
const raw = {
|
||||
iid: 1,
|
||||
title: 'Manual release stage',
|
||||
state: 'opened',
|
||||
[pipelineField]: { status },
|
||||
detailed_merge_status
|
||||
}
|
||||
const info = mapMRInfo(raw, derivePipelineStatus({ status }))
|
||||
expect(info.pipelineStatus, `${pipelineField}: ${status} / ${detailed_merge_status}`).toBe(
|
||||
expected
|
||||
)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
it('builds an MRInfo from a typical glab payload', () => {
|
||||
expect(
|
||||
mapMRInfo(
|
||||
@@ -166,12 +192,14 @@ describe('mapMRInfo', () => {
|
||||
title: 't',
|
||||
state: 'opened',
|
||||
has_conflicts: true,
|
||||
head_pipeline: { status: 'manual' },
|
||||
detailed_merge_status: 'mergeable'
|
||||
},
|
||||
'pending'
|
||||
)
|
||||
expect(info.mergeable).toBe('CONFLICTING')
|
||||
expect(info.mergeStateStatus).toBe('conflict')
|
||||
expect(info.pipelineStatus).toBe('pending')
|
||||
})
|
||||
|
||||
it('marks UNKNOWN when detailed_merge_status is non-mergeable but not a conflict', () => {
|
||||
@@ -186,10 +214,17 @@ describe('mapMRInfo', () => {
|
||||
// GitLab <15.6 lacks detailed_merge_status; legacy positive mergeability keeps actions usable.
|
||||
it('falls back to legacy merge_status when detailed_merge_status is absent', () => {
|
||||
const info = mapMRInfo(
|
||||
{ iid: 1, title: 't', state: 'opened', merge_status: 'can_be_merged' },
|
||||
'success'
|
||||
{
|
||||
iid: 1,
|
||||
title: 't',
|
||||
state: 'opened',
|
||||
merge_status: 'can_be_merged',
|
||||
head_pipeline: { status: 'manual' }
|
||||
},
|
||||
'pending'
|
||||
)
|
||||
expect(info.mergeable).toBe('MERGEABLE')
|
||||
expect(info.pipelineStatus).toBe('pending')
|
||||
})
|
||||
|
||||
it('does not let legacy merge_status override a present detailed_merge_status', () => {
|
||||
|
||||
@@ -76,6 +76,8 @@ type GitLabMRRaw = {
|
||||
description?: string | null
|
||||
target_branch?: string
|
||||
author?: { username?: string | null; avatar_url?: string | null } | null
|
||||
head_pipeline?: { status?: string } | null
|
||||
pipeline?: { status?: string } | null
|
||||
}
|
||||
|
||||
export function mapMRInfo(data: GitLabMRRaw, pipelineStatus: CheckStatus): MRInfo {
|
||||
@@ -86,7 +88,7 @@ export function mapMRInfo(data: GitLabMRRaw, pipelineStatus: CheckStatus): MRInf
|
||||
title: data.title,
|
||||
state: mapMRState(data.state, data.draft, data.title),
|
||||
url: data.web_url ?? data.url ?? '',
|
||||
pipelineStatus,
|
||||
pipelineStatus: settleManualPipelineStatus(data, pipelineStatus),
|
||||
updatedAt: data.updated_at ?? data.updatedAt ?? '',
|
||||
mergeable,
|
||||
...(mergeStateStatus ? { mergeStateStatus } : {}),
|
||||
@@ -99,6 +101,16 @@ export function mapMRInfo(data: GitLabMRRaw, pipelineStatus: CheckStatus): MRInf
|
||||
}
|
||||
}
|
||||
|
||||
// A mergeable manual pipeline has no automatic work outstanding, but its manual stage has not passed.
|
||||
function settleManualPipelineStatus(data: GitLabMRRaw, pipelineStatus: CheckStatus): CheckStatus {
|
||||
const raw = (data.head_pipeline ?? data.pipeline)?.status?.toLowerCase()
|
||||
return raw === 'manual' &&
|
||||
data.detailed_merge_status === 'mergeable' &&
|
||||
data.has_conflicts !== true
|
||||
? 'neutral'
|
||||
: pipelineStatus
|
||||
}
|
||||
|
||||
function deriveMergeable(data: GitLabMRRaw): MRInfo['mergeable'] {
|
||||
if (data.has_conflicts === true) {
|
||||
return 'CONFLICTING'
|
||||
|
||||
@@ -96,22 +96,23 @@ export async function getMergeRequestForBranch(
|
||||
}
|
||||
await acquire()
|
||||
try {
|
||||
if (typeof linkedMRIid === 'number') {
|
||||
const fetchByIid = async (iid: number): Promise<MRInfo> => {
|
||||
const { stdout } = await glabExecFileAsync(
|
||||
[
|
||||
'api',
|
||||
...glabHostnameArgs(projectRef, connectionId),
|
||||
`projects/${encodedProject(projectRef.path)}/merge_requests/${linkedMRIid}?with_merge_status_recheck=true`
|
||||
`projects/${encodedProject(projectRef.path)}/merge_requests/${iid}?with_merge_status_recheck=true`
|
||||
],
|
||||
glabRepoExecOptions(repoPath, connectionId, localGitOptions)
|
||||
)
|
||||
const raw = JSON.parse(stdout) as Parameters<typeof mapMRInfo>[0] & {
|
||||
head_pipeline?: { status?: string } | null
|
||||
pipeline?: { status?: string } | null
|
||||
}
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the single-MR endpoint returns the MR fields and its optional pipeline.
|
||||
const raw = JSON.parse(stdout) as Parameters<typeof mapMRInfo>[0]
|
||||
const pipelineStatus = derivePipelineStatus(raw.head_pipeline ?? raw.pipeline ?? null)
|
||||
return mapMRInfo(raw, pipelineStatus)
|
||||
}
|
||||
if (typeof linkedMRIid === 'number') {
|
||||
return await fetchByIid(linkedMRIid)
|
||||
}
|
||||
if (branchName) {
|
||||
const { stdout } = await glabExecFileAsync(
|
||||
[
|
||||
@@ -122,15 +123,11 @@ export async function getMergeRequestForBranch(
|
||||
],
|
||||
glabRepoExecOptions(repoPath, connectionId, localGitOptions)
|
||||
)
|
||||
const data = JSON.parse(stdout) as (Parameters<typeof mapMRInfo>[0] & {
|
||||
head_pipeline?: { status?: string } | null
|
||||
pipeline?: { status?: string } | null
|
||||
})[]
|
||||
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the branch-list endpoint returns MR summaries; only their identity is used.
|
||||
const data = JSON.parse(stdout) as Parameters<typeof mapMRInfo>[0][]
|
||||
if (Array.isArray(data) && data.length > 0) {
|
||||
const raw = data[0]
|
||||
// Older GitLab uses pipeline instead of head_pipeline.
|
||||
const pipelineStatus = derivePipelineStatus(raw.head_pipeline ?? raw.pipeline ?? null)
|
||||
const info = mapMRInfo(raw, pipelineStatus)
|
||||
// List summaries omit head_pipeline; only detail can supply an authoritative check result.
|
||||
const info = await fetchByIid(data[0].iid ?? data[0].number ?? 0)
|
||||
const hideOnDefaultBranch = await shouldHideNonOpenReviewOnDefaultBranch({
|
||||
state: info.state,
|
||||
reviewNumber: info.number,
|
||||
|
||||
@@ -2,10 +2,26 @@ import { renderToStaticMarkup } from 'react-dom/server'
|
||||
import type { ReactNode } from 'react'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { WorktreeCardStatusSlot } from './WorktreeCardStatusSlot'
|
||||
import type { WorktreeCardPrDisplay } from './worktree-card-pr-display'
|
||||
import { getWorktreeCardPrDisplay, type WorktreeCardPrDisplay } from './worktree-card-pr-display'
|
||||
import { gitlabMrCheckCases } from '../../../../shared/gitlab-mr-checks.test-fixture'
|
||||
import { buildParentPrChecksProjection } from '../right-sidebar/parent-pr-checks-rows'
|
||||
import { FolderWorkspacePrChecksRow } from '../right-sidebar/FolderWorkspacePrChecksRow'
|
||||
import { presentGitLabMRMergeState } from '../right-sidebar/gitlab-mr-merge-state'
|
||||
import { getActiveChecksStatus } from '../right-sidebar/active-checks-status'
|
||||
import { ReviewChecksBadge, ReviewChecksIndicator } from './WorktreeCardMetadataStatusBadges'
|
||||
import { WorktreeReferenceStack } from './WorktreeReferenceStack'
|
||||
import type { WorkspaceReferenceDetailsMap } from './workspace-reference-details'
|
||||
import { getFolderWorkspaceCardPrDisplay } from './folder-workspace-card-pr-display'
|
||||
import { formatHostedReviewLabel } from '../status-bar/hosted-review-label'
|
||||
import { reviewRefreshIntervalMs } from '../../../../shared/review-refresh-policy'
|
||||
import { summarizeWorkspaceReviewChecks } from '../../../../shared/workspace-review-checks'
|
||||
import { getWorkspaceAttachmentKey } from '../../../../shared/workspace-attachment-normalization'
|
||||
import { referenceAttachment, referenceReview } from './workspace-reference-fixtures.test-support'
|
||||
import {
|
||||
referenceAttachment,
|
||||
referenceReview,
|
||||
referenceRepo,
|
||||
referenceWorkspace
|
||||
} from './workspace-reference-fixtures.test-support'
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
status: 'active',
|
||||
@@ -49,6 +65,105 @@ describe('WorktreeCardStatusSlot', () => {
|
||||
status: 'pending'
|
||||
}
|
||||
|
||||
it.each(gitlabMrCheckCases)(
|
||||
'presents the normalized $name without claiming passing',
|
||||
({ review: mapped, cardLabel, tone, rowSummary, mergeLabel, directMergeAvailable }) => {
|
||||
const { status } = mapped
|
||||
const worktree = { ...referenceWorkspace, linkedPR: null, linkedGitLabMR: 7 }
|
||||
const hostedReviewCache = {
|
||||
'local::repo::feature': { data: mapped, fetchedAt: 1, linkedReviewHintKey: 'gitlab:7' }
|
||||
}
|
||||
const context = {
|
||||
worktrees: [worktree],
|
||||
repos: [referenceRepo],
|
||||
settings: null,
|
||||
hostedReviewCache,
|
||||
prCache: {},
|
||||
checksCache: {}
|
||||
}
|
||||
const projection = buildParentPrChecksProjection(context)
|
||||
const card = renderToStaticMarkup(
|
||||
<WorktreeCardStatusSlot
|
||||
worktreeId={worktree.id}
|
||||
showStatus
|
||||
showUnreadAction={false}
|
||||
isUnread={false}
|
||||
unreadTooltip=""
|
||||
onPointerDown={vi.fn()}
|
||||
onToggleUnread={vi.fn()}
|
||||
newCardStyle
|
||||
prDisplay={getWorktreeCardPrDisplay(mapped, null, 7)}
|
||||
/>
|
||||
)
|
||||
expect(card).toContain(cardLabel)
|
||||
expect(card).toContain(tone)
|
||||
expect(card).not.toContain('Passing')
|
||||
const row = renderToStaticMarkup(
|
||||
<FolderWorkspacePrChecksRow
|
||||
row={projection.rows[0]}
|
||||
expanded={false}
|
||||
onToggle={vi.fn()}
|
||||
onLoadCheckDetails={async () => null}
|
||||
/>
|
||||
)
|
||||
expect(row).toContain(rowSummary)
|
||||
expect(row).not.toMatch(/Checks passing|lucide-circle-check/)
|
||||
expect(projection.summary.passing).toBe(0)
|
||||
expect(presentGitLabMRMergeState(mapped)).toMatchObject({
|
||||
label: mergeLabel,
|
||||
directMergeAvailable
|
||||
})
|
||||
expect(
|
||||
getActiveChecksStatus({
|
||||
...context,
|
||||
activeWorktreeId: worktree.id,
|
||||
worktreesByRepo: { repo: [worktree] }
|
||||
})
|
||||
).toBe(status)
|
||||
const item = { provider: 'gitlab', type: 'mr', number: 7 } as const
|
||||
const details = {
|
||||
[getWorkspaceAttachmentKey(item)]: { title: mapped.title, url: mapped.url, review: mapped }
|
||||
} satisfies WorkspaceReferenceDetailsMap
|
||||
expect(summarizeWorkspaceReviewChecks([item], details)).toMatchObject({
|
||||
state: status,
|
||||
passed: 0
|
||||
})
|
||||
expect(
|
||||
renderToStaticMarkup(<WorktreeReferenceStack items={[item]} details={details} />)
|
||||
).toContain(tone)
|
||||
const checkLabel = { failure: 'Failing', pending: 'Pending', neutral: '' }[status]
|
||||
for (const Badge of [ReviewChecksBadge, ReviewChecksIndicator]) {
|
||||
const badge = renderToStaticMarkup(<Badge status={mapped.status} />)
|
||||
expect(badge).toEqual(checkLabel ? expect.stringContaining(`Checks: ${checkLabel}`) : '')
|
||||
}
|
||||
expect(
|
||||
getFolderWorkspaceCardPrDisplay({
|
||||
...context,
|
||||
folderWorkspaceId: 'folder',
|
||||
worktreeMap: new Map([[worktree.id, worktree]]),
|
||||
repoMap: new Map([['repo', referenceRepo]]),
|
||||
worktreeLineageById: {},
|
||||
workspaceLineageByChildKey: {
|
||||
child: {
|
||||
childWorkspaceKey: `worktree:${worktree.id}`,
|
||||
parentWorkspaceKey: 'folder:folder',
|
||||
origin: 'cli',
|
||||
capture: { source: 'env-workspace', confidence: 'inferred' },
|
||||
createdAt: 1
|
||||
}
|
||||
}
|
||||
})
|
||||
).toMatchObject({ state: 'open', status })
|
||||
expect(formatHostedReviewLabel(mapped)).toBe(`MR !7 Open, ${status}`)
|
||||
expect(reviewRefreshIntervalMs({ state: mapped.state, checksStatus: mapped.status })).toBe(
|
||||
120_000
|
||||
)
|
||||
expect(reviewRefreshIntervalMs({ state: 'merged', checksStatus: mapped.status })).toBe(
|
||||
status === 'pending' ? 60_000 : null
|
||||
)
|
||||
}
|
||||
)
|
||||
|
||||
it('uses checks from every attached review rather than the legacy passing review', () => {
|
||||
const items = [referenceAttachment(1), referenceAttachment(2)]
|
||||
const details = Object.fromEntries(
|
||||
|
||||
@@ -0,0 +1,71 @@
|
||||
import type { HostedReviewInfo } from './hosted-review'
|
||||
|
||||
const rawIdentity = { iid: 7, title: 'Release', state: 'opened', has_conflicts: false }
|
||||
const reviewIdentity = {
|
||||
provider: 'gitlab',
|
||||
number: 7,
|
||||
title: 'Release',
|
||||
state: 'open',
|
||||
url: '',
|
||||
updatedAt: ''
|
||||
} satisfies Omit<HostedReviewInfo, 'status' | 'mergeable'>
|
||||
|
||||
export const gitlabMrCheckCases = [
|
||||
{
|
||||
name: 'failed pipeline',
|
||||
raw: {
|
||||
...rawIdentity,
|
||||
head_pipeline: { status: 'failed' },
|
||||
detailed_merge_status: 'ci_must_pass'
|
||||
},
|
||||
review: {
|
||||
...reviewIdentity,
|
||||
status: 'failure',
|
||||
mergeable: 'UNKNOWN',
|
||||
mergeStateStatus: 'ci_must_pass'
|
||||
} satisfies HostedReviewInfo,
|
||||
cardLabel: 'MR checks: Failed',
|
||||
tone: 'text-rose-500/85',
|
||||
rowSummary: 'Checks failing',
|
||||
mergeLabel: 'Checks must pass',
|
||||
directMergeAvailable: false
|
||||
},
|
||||
{
|
||||
name: 'mergeable manual pipeline',
|
||||
raw: {
|
||||
...rawIdentity,
|
||||
head_pipeline: { status: 'manual' },
|
||||
detailed_merge_status: 'mergeable'
|
||||
},
|
||||
review: {
|
||||
...reviewIdentity,
|
||||
status: 'neutral',
|
||||
mergeable: 'MERGEABLE',
|
||||
mergeStateStatus: 'mergeable'
|
||||
} satisfies HostedReviewInfo,
|
||||
cardLabel: 'MR: Open',
|
||||
tone: 'text-emerald-500/80',
|
||||
rowSummary: 'No check signal',
|
||||
mergeLabel: 'Able to merge',
|
||||
directMergeAvailable: true
|
||||
},
|
||||
{
|
||||
name: 'blocked manual pipeline',
|
||||
raw: {
|
||||
...rawIdentity,
|
||||
head_pipeline: { status: 'manual' },
|
||||
detailed_merge_status: 'ci_must_pass'
|
||||
},
|
||||
review: {
|
||||
...reviewIdentity,
|
||||
status: 'pending',
|
||||
mergeable: 'UNKNOWN',
|
||||
mergeStateStatus: 'ci_must_pass'
|
||||
} satisfies HostedReviewInfo,
|
||||
cardLabel: 'MR checks: Pending',
|
||||
tone: 'text-amber-500/85',
|
||||
rowSummary: 'Checks pending',
|
||||
mergeLabel: 'Checks must pass',
|
||||
directMergeAvailable: false
|
||||
}
|
||||
]
|
||||
Reference in New Issue
Block a user