mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 06:20:11 +00:00
refactor: tidy QA CLI — extract functions, validate target, dedup
- Extract fetchIssue/fetchPR/resolveTarget/detectType helpers - Move constants to top (SCRIPT_DIR, RECORD_SCRIPT, DEFAULT_REPO) - Add VALID_TARGETS validation for -t/--target - Shared writeTmpFile/resolveOutputDir/logHeader/exit utilities - Dispatch via runUncommitted/runTarget for clearer flow
This commit is contained in:
snomiao
@@ -16,52 +16,31 @@
|
||||
import { parseArgs } from 'node:util'
|
||||
import { config } from 'dotenv'
|
||||
import { existsSync, mkdirSync, writeFileSync } from 'fs'
|
||||
import { resolve, dirname } from 'path'
|
||||
import { dirname, resolve } from 'path'
|
||||
import { execSync, spawnSync } from 'child_process'
|
||||
import { fileURLToPath } from 'url'
|
||||
|
||||
// ── Load environment from .env.local / .env ──
|
||||
// ── Constants ──
|
||||
|
||||
for (const envFile of ['.env.local', '.env']) {
|
||||
if (existsSync(envFile)) {
|
||||
config({ path: envFile })
|
||||
const SCRIPT_DIR = dirname(fileURLToPath(import.meta.url))
|
||||
const RECORD_SCRIPT = resolve(SCRIPT_DIR, 'qa-record.ts')
|
||||
const DEFAULT_REPO = 'Comfy-Org/ComfyUI_frontend'
|
||||
const VALID_TARGETS = ['head', 'base', 'both'] as const
|
||||
type PrTarget = (typeof VALID_TARGETS)[number]
|
||||
type TargetType = 'issue' | 'pr'
|
||||
|
||||
// ── Load .env.local / .env ──
|
||||
|
||||
for (const f of ['.env.local', '.env']) {
|
||||
if (existsSync(f)) {
|
||||
config({ path: f })
|
||||
break
|
||||
}
|
||||
}
|
||||
|
||||
// ── Parse CLI with node:util parseArgs ──
|
||||
// ── Parse CLI ──
|
||||
|
||||
let values: {
|
||||
target: string
|
||||
uncommitted: boolean
|
||||
url: string
|
||||
ref: string
|
||||
output: string
|
||||
help: boolean
|
||||
}
|
||||
let positionals: string[]
|
||||
|
||||
try {
|
||||
const parsed = parseArgs({
|
||||
args: process.argv.slice(2),
|
||||
options: {
|
||||
target: { type: 'string', short: 't', default: 'head' },
|
||||
uncommitted: { type: 'boolean', default: false },
|
||||
url: { type: 'string', default: '' },
|
||||
ref: { type: 'string', default: '' },
|
||||
output: { type: 'string', short: 'o', default: '' },
|
||||
help: { type: 'boolean', short: 'h', default: false }
|
||||
},
|
||||
allowPositionals: true,
|
||||
strict: true
|
||||
})
|
||||
values = parsed.values as typeof values
|
||||
positionals = parsed.positionals
|
||||
} catch (err) {
|
||||
console.error(`Error: ${err instanceof Error ? err.message : err}\n`)
|
||||
printUsage()
|
||||
process.exit(1)
|
||||
}
|
||||
const { values, positionals } = tryParseArgs()
|
||||
|
||||
if (values.help) {
|
||||
printUsage()
|
||||
@@ -70,116 +49,153 @@ if (values.help) {
|
||||
|
||||
const serverUrl =
|
||||
values.url || process.env.DEV_SERVER_COMFYUI_URL || 'http://127.0.0.1:8188'
|
||||
const prTarget = (values.target ?? 'head') as 'head' | 'base' | 'both'
|
||||
|
||||
// ── Handle --uncommitted mode ──
|
||||
const prTarget = values.target as PrTarget
|
||||
if (!VALID_TARGETS.includes(prTarget)) {
|
||||
console.error(
|
||||
`Invalid --target "${prTarget}". Must be one of: ${VALID_TARGETS.join(', ')}`
|
||||
)
|
||||
process.exit(1)
|
||||
}
|
||||
|
||||
// ── Dispatch by mode ──
|
||||
|
||||
if (values.uncommitted) {
|
||||
const diff = execSync('git diff && git diff --staged', {
|
||||
encoding: 'utf-8'
|
||||
})
|
||||
runUncommitted()
|
||||
} else {
|
||||
const input = positionals[0]
|
||||
if (!input) {
|
||||
printUsage()
|
||||
process.exit(1)
|
||||
}
|
||||
runTarget(input)
|
||||
}
|
||||
|
||||
// ── Mode: uncommitted changes ──
|
||||
|
||||
function runUncommitted(): never {
|
||||
const diff = shell('git diff && git diff --staged')
|
||||
if (!diff.trim()) {
|
||||
console.error('No uncommitted changes found')
|
||||
process.exit(1)
|
||||
}
|
||||
|
||||
const outputDir = values.output
|
||||
? resolve(values.output)
|
||||
: resolve('.comfy-qa/local')
|
||||
mkdirSync(outputDir, { recursive: true })
|
||||
const outputDir = resolveOutputDir('.comfy-qa/local')
|
||||
const diffFile = writeTmpFile(outputDir, 'uncommitted.diff', diff)
|
||||
|
||||
const tmpDir = resolve(outputDir, '.tmp')
|
||||
mkdirSync(tmpDir, { recursive: true })
|
||||
const diffFile = resolve(tmpDir, 'uncommitted.diff')
|
||||
writeFileSync(diffFile, diff)
|
||||
|
||||
console.warn('QA target: uncommitted changes')
|
||||
console.warn(`Output: ${outputDir}`)
|
||||
console.warn(`Server: ${serverUrl}`)
|
||||
|
||||
const exitCode = runQaRecord('after', diffFile, outputDir)
|
||||
printSummary(outputDir)
|
||||
process.exit(exitCode)
|
||||
logHeader({ label: 'uncommitted changes', outputDir })
|
||||
const code = runQaRecord('after', diffFile, outputDir)
|
||||
exit(code, outputDir)
|
||||
}
|
||||
|
||||
// ── Parse positional target ──
|
||||
// ── Mode: issue or PR by number/URL ──
|
||||
|
||||
const input = positionals[0]
|
||||
if (!input) {
|
||||
function runTarget(input: string): never {
|
||||
const { targetType, number, repo } = resolveTarget(input)
|
||||
const outputDir = resolveOutputDir(`.comfy-qa/${number}`)
|
||||
|
||||
logHeader({
|
||||
label: `${targetType} #${number} (${repo})`,
|
||||
outputDir,
|
||||
extra: targetType === 'pr' ? `Target: ${prTarget}` : undefined
|
||||
})
|
||||
|
||||
const diffFile =
|
||||
targetType === 'issue'
|
||||
? fetchIssue(number, repo, outputDir)
|
||||
: fetchPR(number, repo, outputDir)
|
||||
|
||||
let exitCode: number
|
||||
if (targetType === 'issue') {
|
||||
exitCode = runQaRecord('reproduce', diffFile, outputDir)
|
||||
} else if (prTarget === 'both') {
|
||||
exitCode = runPrBoth(diffFile, outputDir)
|
||||
} else if (prTarget === 'base') {
|
||||
exitCode = runQaRecord('before', diffFile, outputDir)
|
||||
} else {
|
||||
exitCode = runQaRecord('after', diffFile, outputDir)
|
||||
}
|
||||
|
||||
exit(exitCode, outputDir)
|
||||
}
|
||||
|
||||
// ── PR both phases ──
|
||||
|
||||
function runPrBoth(diffFile: string, outputDir: string): number {
|
||||
console.warn('\n=== Phase 1: Reproduce bug on base ===')
|
||||
const baseDir = resolve(outputDir, 'base')
|
||||
mkdirSync(baseDir, { recursive: true })
|
||||
const baseCode = runQaRecord('before', diffFile, baseDir)
|
||||
if (baseCode !== 0) {
|
||||
console.warn('Base phase failed, continuing to head...')
|
||||
}
|
||||
|
||||
console.warn('\n=== Phase 2: Demonstrate fix on head ===')
|
||||
const headDir = resolve(outputDir, 'head')
|
||||
mkdirSync(headDir, { recursive: true })
|
||||
return runQaRecord('after', diffFile, headDir)
|
||||
}
|
||||
|
||||
// ── Target resolution ──
|
||||
|
||||
function resolveTarget(input: string): {
|
||||
targetType: TargetType
|
||||
number: string
|
||||
repo: string
|
||||
} {
|
||||
const urlMatch = input.match(
|
||||
/github\.com\/([^/]+\/[^/]+)\/(issues|pull)\/(\d+)/
|
||||
)
|
||||
|
||||
if (urlMatch) {
|
||||
return {
|
||||
repo: urlMatch[1],
|
||||
targetType: urlMatch[2] === 'pull' ? 'pr' : 'issue',
|
||||
number: urlMatch[3]
|
||||
}
|
||||
}
|
||||
|
||||
if (/^\d+$/.test(input)) {
|
||||
return {
|
||||
repo: DEFAULT_REPO,
|
||||
targetType: detectType(input, DEFAULT_REPO),
|
||||
number: input
|
||||
}
|
||||
}
|
||||
|
||||
console.error(`Cannot parse target: ${input}`)
|
||||
console.error('Expected a GitHub URL or issue/PR number')
|
||||
printUsage()
|
||||
process.exit(1)
|
||||
}
|
||||
|
||||
type TargetType = 'issue' | 'pr'
|
||||
|
||||
let targetType: TargetType | undefined
|
||||
let number: string
|
||||
let repo = 'Comfy-Org/ComfyUI_frontend'
|
||||
|
||||
const ghUrlMatch = input.match(
|
||||
/github\.com\/([^/]+\/[^/]+)\/(issues|pull)\/(\d+)/
|
||||
)
|
||||
|
||||
if (ghUrlMatch) {
|
||||
repo = ghUrlMatch[1]
|
||||
targetType = ghUrlMatch[2] === 'pull' ? 'pr' : 'issue'
|
||||
number = ghUrlMatch[3]
|
||||
} else if (/^\d+$/.test(input)) {
|
||||
number = input
|
||||
} else {
|
||||
console.error(`Cannot parse target: ${input}`)
|
||||
console.error('Expected a GitHub URL or number')
|
||||
process.exit(1)
|
||||
}
|
||||
|
||||
// Auto-detect issue vs PR when only a number is given
|
||||
if (!targetType) {
|
||||
function detectType(number: string, repo: string): TargetType {
|
||||
try {
|
||||
execSync(`gh pr view ${number} --repo ${repo} --json number`, {
|
||||
encoding: 'utf-8',
|
||||
timeout: 15000,
|
||||
stdio: ['pipe', 'pipe', 'pipe']
|
||||
})
|
||||
targetType = 'pr'
|
||||
return 'pr'
|
||||
} catch {
|
||||
targetType = 'issue'
|
||||
return 'issue'
|
||||
}
|
||||
}
|
||||
|
||||
// ── Set up output directory ──
|
||||
// ── Data fetching ──
|
||||
|
||||
const outputDir = values.output
|
||||
? resolve(values.output)
|
||||
: resolve(`.comfy-qa/${number}`)
|
||||
|
||||
mkdirSync(outputDir, { recursive: true })
|
||||
|
||||
console.warn(`QA target: ${targetType} #${number} (${repo})`)
|
||||
console.warn(`Output: ${outputDir}`)
|
||||
console.warn(`Server: ${serverUrl}`)
|
||||
if (values.ref) console.warn(`Ref: ${values.ref}`)
|
||||
if (targetType === 'pr') console.warn(`Target: ${prTarget}`)
|
||||
|
||||
// ── Fetch issue/PR data via gh CLI ──
|
||||
|
||||
const tmpDir = resolve(outputDir, '.tmp')
|
||||
mkdirSync(tmpDir, { recursive: true })
|
||||
|
||||
let diffFile: string
|
||||
|
||||
if (targetType === 'issue') {
|
||||
function fetchIssue(number: string, repo: string, outputDir: string): string {
|
||||
console.warn(`Fetching issue #${number}...`)
|
||||
const body = execSync(
|
||||
`gh issue view ${number} --repo ${repo} --json title,body,labels --jq '"Title: " + .title + "\\n\\nLabels: " + ([.labels[].name] | join(", ")) + "\\n\\n" + .body'`,
|
||||
{ encoding: 'utf-8', timeout: 30000 }
|
||||
const body = shell(
|
||||
`gh issue view ${number} --repo ${repo} --json title,body,labels --jq '"Title: " + .title + "\\n\\nLabels: " + ([.labels[].name] | join(", ")) + "\\n\\n" + .body'`
|
||||
)
|
||||
diffFile = resolve(tmpDir, `issue-${number}.txt`)
|
||||
writeFileSync(diffFile, body)
|
||||
} else {
|
||||
return writeTmpFile(outputDir, `issue-${number}.txt`, body)
|
||||
}
|
||||
|
||||
function fetchPR(number: string, repo: string, outputDir: string): string {
|
||||
console.warn(`Fetching PR #${number}...`)
|
||||
const prJson = execSync(
|
||||
`gh pr view ${number} --repo ${repo} --json title,body,baseRefName,headRefName,baseRefOid,headRefOid`,
|
||||
{ encoding: 'utf-8', timeout: 30000 }
|
||||
const prJson = shell(
|
||||
`gh pr view ${number} --repo ${repo} --json title,body,baseRefName,headRefName,baseRefOid,headRefOid`
|
||||
)
|
||||
const pr = JSON.parse(prJson) as {
|
||||
title: string
|
||||
@@ -189,26 +205,20 @@ if (targetType === 'issue') {
|
||||
baseRefOid: string
|
||||
headRefOid: string
|
||||
}
|
||||
|
||||
console.warn(` Base: ${pr.baseRefName} (${pr.baseRefOid.slice(0, 8)})`)
|
||||
console.warn(` Head: ${pr.headRefName} (${pr.headRefOid.slice(0, 8)})`)
|
||||
|
||||
let diff = ''
|
||||
try {
|
||||
diff = execSync(`gh pr diff ${number} --repo ${repo}`, {
|
||||
encoding: 'utf-8',
|
||||
timeout: 30000
|
||||
})
|
||||
diff = shell(`gh pr diff ${number} --repo ${repo}`)
|
||||
} catch {
|
||||
console.warn('Could not fetch PR diff (PR may be closed/merged)')
|
||||
console.warn('Could not fetch PR diff')
|
||||
}
|
||||
diffFile = resolve(tmpDir, `pr-${number}.txt`)
|
||||
writeFileSync(
|
||||
diffFile,
|
||||
`Title: ${pr.title}\n\n${pr.body}\n\n--- DIFF ---\n\n${diff}`
|
||||
)
|
||||
|
||||
writeFileSync(
|
||||
resolve(tmpDir, 'refs.json'),
|
||||
writeTmpFile(
|
||||
outputDir,
|
||||
'refs.json',
|
||||
JSON.stringify(
|
||||
{
|
||||
base: { ref: pr.baseRefName, sha: pr.baseRefOid },
|
||||
@@ -218,79 +228,116 @@ if (targetType === 'issue') {
|
||||
2
|
||||
)
|
||||
)
|
||||
|
||||
return writeTmpFile(
|
||||
outputDir,
|
||||
`pr-${number}.txt`,
|
||||
`Title: ${pr.title}\n\n${pr.body}\n\n--- DIFF ---\n\n${diff}`
|
||||
)
|
||||
}
|
||||
|
||||
// ── Run QA ──
|
||||
|
||||
let exitCode = 0
|
||||
|
||||
if (targetType === 'issue') {
|
||||
exitCode = runQaRecord('reproduce', diffFile, outputDir)
|
||||
} else if (prTarget === 'both') {
|
||||
console.warn('\n=== Phase 1: Reproduce bug on base ===')
|
||||
const baseDir = resolve(outputDir, 'base')
|
||||
mkdirSync(baseDir, { recursive: true })
|
||||
const baseCode = runQaRecord('before', diffFile, baseDir)
|
||||
if (baseCode !== 0) {
|
||||
console.warn('Base phase failed, continuing to head phase...')
|
||||
}
|
||||
|
||||
console.warn('\n=== Phase 2: Demonstrate fix on head ===')
|
||||
const headDir = resolve(outputDir, 'head')
|
||||
mkdirSync(headDir, { recursive: true })
|
||||
exitCode = runQaRecord('after', diffFile, headDir)
|
||||
} else if (prTarget === 'base') {
|
||||
exitCode = runQaRecord('before', diffFile, outputDir)
|
||||
} else {
|
||||
exitCode = runQaRecord('after', diffFile, outputDir)
|
||||
}
|
||||
|
||||
printSummary(outputDir)
|
||||
process.exit(exitCode)
|
||||
|
||||
// ── Helpers ──
|
||||
// ── QA record runner ──
|
||||
|
||||
function runQaRecord(
|
||||
qaMode: string,
|
||||
qaDiffFile: string,
|
||||
qaOutputDir: string
|
||||
mode: string,
|
||||
diffFile: string,
|
||||
outputDir: string
|
||||
): number {
|
||||
const scriptDir = dirname(fileURLToPath(import.meta.url))
|
||||
const recordScript = resolve(scriptDir, 'qa-record.ts')
|
||||
|
||||
const cmd = [
|
||||
console.warn(`\nStarting QA ${mode} mode...\n`)
|
||||
const r = spawnSync(
|
||||
'pnpm',
|
||||
'exec',
|
||||
'tsx',
|
||||
recordScript,
|
||||
'--mode',
|
||||
qaMode,
|
||||
'--diff',
|
||||
qaDiffFile,
|
||||
'--output-dir',
|
||||
qaOutputDir,
|
||||
'--url',
|
||||
serverUrl
|
||||
]
|
||||
|
||||
console.warn(`\nStarting QA ${qaMode} mode...\n`)
|
||||
|
||||
const r = spawnSync(cmd[0], cmd.slice(1), {
|
||||
stdio: 'inherit',
|
||||
env: process.env
|
||||
})
|
||||
[
|
||||
'exec',
|
||||
'tsx',
|
||||
RECORD_SCRIPT,
|
||||
'--mode',
|
||||
mode,
|
||||
'--diff',
|
||||
diffFile,
|
||||
'--output-dir',
|
||||
outputDir,
|
||||
'--url',
|
||||
serverUrl
|
||||
],
|
||||
{ stdio: 'inherit', env: process.env }
|
||||
)
|
||||
return r.status ?? 1
|
||||
}
|
||||
|
||||
function printSummary(dir: string) {
|
||||
// ── Utilities ──
|
||||
|
||||
function shell(cmd: string): string {
|
||||
return execSync(cmd, { encoding: 'utf-8', timeout: 30000 })
|
||||
}
|
||||
|
||||
function writeTmpFile(
|
||||
outputDir: string,
|
||||
filename: string,
|
||||
content: string
|
||||
): string {
|
||||
const tmpDir = resolve(outputDir, '.tmp')
|
||||
mkdirSync(tmpDir, { recursive: true })
|
||||
const filePath = resolve(tmpDir, filename)
|
||||
writeFileSync(filePath, content)
|
||||
return filePath
|
||||
}
|
||||
|
||||
function resolveOutputDir(defaultPath: string): string {
|
||||
const dir = values.output ? resolve(values.output) : resolve(defaultPath)
|
||||
mkdirSync(dir, { recursive: true })
|
||||
return dir
|
||||
}
|
||||
|
||||
function logHeader(opts: { label: string; outputDir: string; extra?: string }) {
|
||||
console.warn(`QA target: ${opts.label}`)
|
||||
console.warn(`Output: ${opts.outputDir}`)
|
||||
console.warn(`Server: ${serverUrl}`)
|
||||
if (values.ref) console.warn(`Ref: ${values.ref}`)
|
||||
if (opts.extra) console.warn(opts.extra)
|
||||
}
|
||||
|
||||
function exit(code: number, outputDir: string): never {
|
||||
console.warn('\n=== QA Complete ===')
|
||||
console.warn(`Results: ${dir}`)
|
||||
console.warn(`Results: ${outputDir}`)
|
||||
try {
|
||||
const files = execSync(`ls -la "${dir}"`, { encoding: 'utf-8' })
|
||||
console.warn(files)
|
||||
console.warn(shell(`ls -la "${outputDir}"`))
|
||||
} catch {
|
||||
// not critical
|
||||
}
|
||||
process.exit(code)
|
||||
}
|
||||
|
||||
function tryParseArgs() {
|
||||
try {
|
||||
const parsed = parseArgs({
|
||||
args: process.argv.slice(2),
|
||||
options: {
|
||||
target: { type: 'string', short: 't', default: 'head' },
|
||||
uncommitted: { type: 'boolean', default: false },
|
||||
url: { type: 'string', default: '' },
|
||||
ref: { type: 'string', default: '' },
|
||||
output: { type: 'string', short: 'o', default: '' },
|
||||
help: { type: 'boolean', short: 'h', default: false }
|
||||
},
|
||||
allowPositionals: true,
|
||||
strict: true
|
||||
})
|
||||
return {
|
||||
values: parsed.values as {
|
||||
target: string
|
||||
uncommitted: boolean
|
||||
url: string
|
||||
ref: string
|
||||
output: string
|
||||
help: boolean
|
||||
},
|
||||
positionals: parsed.positionals
|
||||
}
|
||||
} catch (err) {
|
||||
console.error(`Error: ${err instanceof Error ? err.message : err}\n`)
|
||||
printUsage()
|
||||
process.exit(1)
|
||||
}
|
||||
}
|
||||
|
||||
function printUsage() {
|
||||
@@ -302,7 +349,7 @@ Usage:
|
||||
pnpm qa --uncommitted
|
||||
|
||||
Targets:
|
||||
10253 Number (auto-detects issue vs PR)
|
||||
10253 Number (auto-detects issue vs PR via gh CLI)
|
||||
https://github.com/Comfy-Org/ComfyUI_frontend/issues/10253
|
||||
https://github.com/Comfy-Org/ComfyUI_frontend/pull/10270
|
||||
|
||||
|
||||
Reference in New Issue
Block a user