Skip to content
Draft
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
158 changes: 158 additions & 0 deletions packages/app/src/cli/commands/app/validate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,18 @@ import {linkedAppContext} from '../../services/app-context.js'
import {validateApp} from '../../services/validate.js'
import {testAppLinked} from '../../models/app/app.test-data.js'
import {describe, expect, test, vi} from 'vitest'
import {AbortError} from '@shopify/cli-kit/node/error'
import {outputResult} from '@shopify/cli-kit/node/output'

vi.mock('../../services/app-context.js')
vi.mock('../../services/validate.js')
vi.mock('@shopify/cli-kit/node/output', async (importOriginal) => {
const actual = await importOriginal<typeof import('@shopify/cli-kit/node/output')>()
return {
...actual,
outputResult: vi.fn(),
}
})

describe('app validate command', () => {
test('calls validateApp with json: false by default', async () => {
Expand Down Expand Up @@ -46,4 +55,153 @@ describe('app validate command', () => {
// Then
expect(validateApp).toHaveBeenCalledWith(app, {json: true})
})

test('outputs json issues when app loading aborts before validateApp runs', async () => {
// Given
vi.mocked(linkedAppContext).mockRejectedValue(
new AbortError('Validation errors in /tmp/shopify.app.toml:\n\n• [name]: String is required'),
)

// When / Then
await Validate.run(['--json', '--path=/tmp/app'], import.meta.url).catch(() => {})
expect(outputResult).toHaveBeenCalledWith(
JSON.stringify(
{
valid: false,
issues: [
{
filePath: '/tmp/shopify.app.toml',
path: ['name'],
pathString: 'name',
message: 'String is required',
},
],
},
null,
2,
),
)
expect(validateApp).not.toHaveBeenCalled()
})

test('outputs json issues when app loading aborts with ansi-colored structured text', async () => {
// Given
vi.mocked(linkedAppContext).mockRejectedValue(
new AbortError(
'\u001b[1m\u001b[91mValidation errors\u001b[39m\u001b[22m in /tmp/shopify.app.toml:\n\n• [name]: String is required',
),
)

// When / Then
await Validate.run(['--json', '--path=/tmp/app'], import.meta.url).catch(() => {})
expect(outputResult).toHaveBeenCalledWith(
JSON.stringify(
{
valid: false,
issues: [
{
filePath: '/tmp/shopify.app.toml',
path: ['name'],
pathString: 'name',
message: 'String is required',
},
],
},
null,
2,
),
)
expect(validateApp).not.toHaveBeenCalled()
})

test('preserves a root json issue when contextual text precedes structured validation errors', async () => {
// Given
vi.mocked(linkedAppContext).mockRejectedValue(
new AbortError(
'Could not infer extension handle\n\nValidation errors in /tmp/shopify.app.toml:\n\n• [name]: String is required',
),
)

// When / Then
await Validate.run(['--json', '--path=/tmp/app'], import.meta.url).catch(() => {})
expect(outputResult).toHaveBeenCalledWith(
JSON.stringify(
{
valid: false,
issues: [
{
filePath: '/tmp/shopify.app.toml',
path: ['name'],
pathString: 'name',
message: 'String is required',
},
{
filePath: '/tmp/shopify.app.toml',
path: [],
pathString: 'root',
message:
'Could not infer extension handle\n\nValidation errors in /tmp/shopify.app.toml:\n\n• [name]: String is required',
},
],
},
null,
2,
),
)
expect(validateApp).not.toHaveBeenCalled()
})

test('parses structured validation errors for windows-style paths', async () => {
// Given
vi.mocked(linkedAppContext).mockRejectedValue(
new AbortError('Validation errors in C:\\tmp\\shopify.app.toml:\n\n• [name]: String is required'),
)

// When / Then
await Validate.run(['--json', '--path=/tmp/app'], import.meta.url).catch(() => {})
expect(outputResult).toHaveBeenCalledWith(
JSON.stringify(
{
valid: false,
issues: [
{
filePath: 'C:\\tmp\\shopify.app.toml',
path: ['name'],
pathString: 'name',
message: 'String is required',
},
],
},
null,
2,
),
)
expect(validateApp).not.toHaveBeenCalled()
})

test('outputs a root json issue when app loading aborts with a non-structured message', async () => {
// Given
vi.mocked(linkedAppContext).mockRejectedValue(new AbortError("Couldn't find an app toml file at /tmp/app"))

// When / Then
await Validate.run(['--json', '--path=/tmp/app'], import.meta.url).catch(() => {})
expect(outputResult).toHaveBeenCalledWith(
JSON.stringify(
{
valid: false,
issues: [
{
filePath: '/tmp/app',
path: [],
pathString: 'root',
message: "Couldn't find an app toml file at /tmp/app",
},
],
},
null,
2,
),
)
expect(validateApp).not.toHaveBeenCalled()
})
})
101 changes: 92 additions & 9 deletions packages/app/src/cli/commands/app/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,71 @@ import {validateApp} from '../../services/validate.js'
import AppLinkedCommand, {AppLinkedCommandOutput} from '../../utilities/app-linked-command.js'
import {linkedAppContext} from '../../services/app-context.js'
import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli'
import {AbortError, AbortSilentError} from '@shopify/cli-kit/node/error'
import {outputResult, unstyled} from '@shopify/cli-kit/node/output'

import type {AppValidationIssue} from '../../models/app/error-parsing.js'

function parseIssuePath(pathString: string): (string | number)[] {
if (pathString === 'root') return []

return pathString.split('.').map((segment) => {
const parsed = Number(segment)
return Number.isInteger(parsed) && String(parsed) === segment ? parsed : segment
})
}

function toRootIssue(filePath: string, message: string): AppValidationIssue {
return {
filePath,
path: [],
pathString: 'root',
message,
}
}

function hasMeaningfulPrefix(prefix: string): boolean {
const normalizedPrefix = prefix.trim()
return normalizedPrefix !== '' && normalizedPrefix !== 'App configuration is not valid'
}

function toJsonIssuesFromAbortError(error: AbortError, fallbackFilePath: string): AppValidationIssue[] {
const message = unstyled(error.message).trim()
const marker = 'Validation errors in '
const markerIndex = message.indexOf(marker)

if (markerIndex === -1) {
return [toRootIssue(fallbackFilePath, message)]
}

const bodyStartIndex = message.indexOf('\n\n', markerIndex)
if (bodyStartIndex === -1) {
return [toRootIssue(fallbackFilePath, message)]
}

const filePathLine = message.slice(markerIndex + marker.length, bodyStartIndex)
if (!filePathLine.endsWith(':')) {
return [toRootIssue(fallbackFilePath, message)]
}

const filePath = filePathLine.slice(0, -1)
const body = message.slice(bodyStartIndex + 2)
const issues = Array.from(body.matchAll(/^• \[([^\]]+)\]: (.+)$/gm)).map((captures) => {
const pathString = captures[1]!
const issueMessage = captures[2]!

return {
filePath,
path: parseIssuePath(pathString),
pathString,
message: issueMessage,
}
})

if (issues.length === 0) return [toRootIssue(filePath, message)]
if (hasMeaningfulPrefix(message.slice(0, markerIndex))) return [...issues, toRootIssue(filePath, message)]
return issues
}

export default class Validate extends AppLinkedCommand {
static summary = 'Validate your app configuration and extensions.'
Expand All @@ -20,16 +85,34 @@ export default class Validate extends AppLinkedCommand {
public async run(): Promise<AppLinkedCommandOutput> {
const {flags} = await this.parse(Validate)

const {app} = await linkedAppContext({
directory: flags.path,
clientId: flags['client-id'],
forceRelink: flags.reset,
userProvidedConfigName: flags.config,
unsafeReportMode: true,
})
try {
const {app} = await linkedAppContext({
directory: flags.path,
clientId: flags['client-id'],
forceRelink: flags.reset,
userProvidedConfigName: flags.config,
unsafeReportMode: true,
})

await validateApp(app, {json: flags.json})

await validateApp(app, {json: flags.json})
return {app}
} catch (error) {
if (flags.json && error instanceof AbortError) {
outputResult(
JSON.stringify(
{
valid: false,
issues: toJsonIssuesFromAbortError(error, flags.path),
},
null,
2,
),
)
throw new AbortSilentError()
}

return {app}
throw error
}
}
}
Loading
Loading