From 7f3bb834e7e8f5abf7ab517593897023c2deb8a9 Mon Sep 17 00:00:00 2001 From: "Alex C. Huber" <91097647+alexchuber@users.noreply.github.com> Date: Thu, 17 Sep 2026 17:31:50 -0400 Subject: [PATCH 1/5] feat: add document validation block Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- package.json | 1 + packages/cli/README.md | 10 +- packages/cli/src/pipeline.ts | 7 +- packages/core/docs/blocks.md | 6 + packages/core/docs/usage.md | 40 ++++++ packages/core/package.json | 1 + packages/core/src/blocks/validateBlock.ts | 80 ++++++++++++ packages/core/src/index.ts | 1 + packages/core/src/types/gltf-validator.d.ts | 30 +++++ packages/core/vite.config.ts | 7 +- pnpm-lock.yaml | 11 ++ tests/bundle/browserConsumerBundle.test.ts | 6 +- tests/e2e/cli.test.ts | 46 ++++++- tests/integration/validate.test.ts | 130 ++++++++++++++++++++ tests/unit/validateBlock.test.ts | 110 +++++++++++++++++ 15 files changed, 480 insertions(+), 6 deletions(-) create mode 100644 packages/core/src/blocks/validateBlock.ts create mode 100644 packages/core/src/types/gltf-validator.d.ts create mode 100644 tests/integration/validate.test.ts create mode 100644 tests/unit/validateBlock.test.ts diff --git a/package.json b/package.json index 59c066f..9cb1b20 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,7 @@ "eslint-config-prettier": "^10.1.8", "eslint-plugin-prettier": "^5.5.6", "globals": "^17.7.0", + "gltf-validator": "2.0.0-dev.3.10", "prettier": "^3.9.6", "typedoc": "^0.28.20", "typescript": "^6.0.3", diff --git a/packages/cli/README.md b/packages/cli/README.md index 5ed256a..ccc69f5 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -13,6 +13,7 @@ A pipeline is a sequence of operations applied to a 3D asset. The CLI allows you ```sh node-assets pipeline input.gltf output.glb node-assets pipeline input.glb ktx2 draco output.glb +node-assets pipeline input.glb validate output.glb ``` The command syntax is: @@ -25,10 +26,17 @@ node-assets pipeline [operation...] [--stats] [--benchmark] | --------- | -------------------------- | | Input | `.gltf`, `.glb` | | Output | `.glb` | -| Operation | `draco`, `meshopt`, `ktx2` | +| Operation | `draco`, `meshopt`, `ktx2`, `validate` | Without specifying operations, the CLI reads the input and writes it back out as the target output format. +`validate` checks the current document, including buffers and images, with the +Khronos glTF Validator. It prints grouped diagnostics using the input path as the +label. Warnings, informational issues, and hints do not fail the run; +`UNSUPPORTED_EXTENSION` is ignored. Any error stops the pipeline with exit code 1 +before writing output. Place `validate` before or after other operations to check +the document at that point; it does not validate the original file's packaging. + Run `node-assets --help` for command help or `node-assets --version` for the installed version. diff --git a/packages/cli/src/pipeline.ts b/packages/cli/src/pipeline.ts index c4da63f..d61a887 100644 --- a/packages/cli/src/pipeline.ts +++ b/packages/cli/src/pipeline.ts @@ -17,6 +17,11 @@ export function getPipelineDefinitions() { }, ], operations: [ + { + name: "validate", + description: "Validate the document and fail on errors", + create: (library: typeof NodeAssets, path: string) => new library.ValidateBlock({ uri: path }), + }, { name: "draco", description: "Compress geometry with Draco", @@ -66,7 +71,7 @@ export async function createPipelineAsync({ inputPath, outputPath, blockNames }: const source = inputDefinition.create(library, inputPath); let previous = source.output; for (const transform of transforms) { - const block = transform.create(library); + const block = transform.create(library, inputPath); previous.connectTo(block.input); previous = block.output; } diff --git a/packages/core/docs/blocks.md b/packages/core/docs/blocks.md index 5ba394f..88241a4 100644 --- a/packages/core/docs/blocks.md +++ b/packages/core/docs/blocks.md @@ -26,6 +26,12 @@ # Transforms +- `ValidateBlock` + - Input: `Document` + - Output: the same `Document` + - Uses: `gltf-validator`; execution-scoped `PlatformIO` + - Behavior: Serializes the current document and validates its JSON, buffers, and images. Logs grouped errors, warnings (including informational issues), and hints with their locations. Throws on any errors; otherwise prints a success message. Ignores `UNSUPPORTED_EXTENSION` and does not truncate diagnostics. + - Options: `uri` sets the diagnostic label (default: `scene.glb`); it is not loaded. - `EncodeKTX2Block` - Input: `Document` - Output: `Document` (but in future should be type that locks images and/or textures) diff --git a/packages/core/docs/usage.md b/packages/core/docs/usage.md index d154f5c..c3c2d05 100644 --- a/packages/core/docs/usage.md +++ b/packages/core/docs/usage.md @@ -16,6 +16,46 @@ const asset = new NodeAsset({ const result = await asset.executeAsync(); ``` +# Example: Validating a document + +Insert a `ValidateBlock` wherever the current document should be checked by the +Khronos glTF Validator, or use it as the output block to return the validated +`Document`. + +```ts +const source = new GltfInputBlock({ input: "box%20(1).glb" }); +const validate = new ValidateBlock({ uri: "box%20(1).glb" }); +const destination = new GltfOutputBlock(); + +source.output.connectTo(validate.input); +validate.output.connectTo(destination.input); + +const asset = new NodeAsset({ name: "validated-gltf", outputBlock: destination }); +const result = await asset.executeAsync(); +``` + +Validation checks a serialized copy of the current document, including its +buffers and images, rather than the original source file. It returns the same +`Document` reference on success. The optional `uri` is a display label, not a file +to load; it defaults to `scene.glb` because documents do not retain source paths. + +If there are no errors, the block prints a check mark followed by ` is valid` +using `console.log`. Diagnostics are grouped by severity, code, and message, with +each location on an indented `at` line. Errors are labeled `[Error]`, warnings and +informational issues are labeled `[Warning]`, and hints are labeled `[Hint]`. +Errors come first, then warnings, informational issues, and hints. All reported +diagnostics are logged, even when validation fails. Any validation error rejects +the execution, preventing downstream blocks from running. + +`UNSUPPORTED_EXTENSION` issues are ignored. Other diagnostics are not truncated, +so earlier warnings cannot hide a later error. + +The CLI supports the same block: + +```sh +node-assets pipeline input.glb validate output.glb +``` + # Example: CLI run reports ```sh diff --git a/packages/core/package.json b/packages/core/package.json index 8b3edb8..ac069fd 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -54,6 +54,7 @@ "@types/draco3dgltf": "1.4.3", "babylonpress-ktx2-encoder": "0.6.0", "draco3dgltf": "1.5.7", + "gltf-validator": "2.0.0-dev.3.10", "meshoptimizer": "1.2.0", "sharp": "0.35.4" } diff --git a/packages/core/src/blocks/validateBlock.ts b/packages/core/src/blocks/validateBlock.ts new file mode 100644 index 0000000..8543020 --- /dev/null +++ b/packages/core/src/blocks/validateBlock.ts @@ -0,0 +1,80 @@ +import { ALL_EXTENSIONS } from "@gltf-transform/extensions"; +import type { ValidationIssue } from "gltf-validator"; + +import { GltfDocumentType } from "../connectionPoints/gltfDocument"; +import { UrlType } from "../connectionPoints/url"; +import { PlatformIOResource } from "../resources/platformIOResource"; +import { Block, type BlockOptions } from "./block"; +import { defineBlock, value } from "./blockDefinition"; + +const ValidateBlockDefinition = /* @__PURE__ */ defineBlock({ + type: "transform.validate", + input: GltfDocumentType, + output: GltfDocumentType, + config: { + uri: /* @__PURE__ */ value(UrlType, "scene.glb"), + }, + resources: { + io: PlatformIOResource, + }, + runAsync: async (document, { uri }, { io }) => { + const { validateString } = await import("gltf-validator"); + const { json, resources } = await io.registerExtensions(ALL_EXTENSIONS).writeJSON(document); + const report = await validateString(JSON.stringify(json), { + uri, + ignoredIssues: ["UNSUPPORTED_EXTENSION"], + maxIssues: 0, + externalResourceFunction: async (resourceUri) => { + const data = resources[resourceUri] ?? resources[decodeURIComponent(resourceUri)]; + if (data === undefined) { + throw new Error(`Missing serialized resource "${resourceUri}".`); + } + return data; + }, + }); + + const diagnostics = formatIssues(report.issues.messages); + if (report.issues.numErrors === 0) { + diagnostics.unshift(`\u2705 ${uri} is valid`); + } + console.log(diagnostics.join("\n\n")); + + if (report.issues.numErrors > 0) { + throw new Error(`glTF validation failed for "${uri}" with ${report.issues.numErrors} error(s).`); + } + return document; + }, +}); + +/** Options for the block, its input document, and a diagnostic URI label (default: scene.glb). */ +export type ValidateBlockOptions = BlockOptions; + +/** Validates a document, logs grouped diagnostics, and rejects execution on validation errors. */ +export class ValidateBlock extends Block { + public constructor(options?: ValidateBlockOptions) { + super(ValidateBlockDefinition, options); + } +} + +function formatIssues(issues: readonly ValidationIssue[]): string[] { + const groups = new Map(); + for (const issue of [...issues].reverse()) { + const key = JSON.stringify([issue.severity, issue.code, issue.message]); + let group = groups.get(key); + if (group === undefined) { + group = { issue, locations: [] }; + groups.set(key, group); + } + if (issue.pointer !== undefined) { + group.locations.push(` at ${issue.pointer || "/"}`); + } else if (issue.offset !== undefined) { + group.locations.push(` at byte ${issue.offset}`); + } + } + return [...groups.values()] + .sort((a, b) => a.issue.severity - b.issue.severity) + .map(({ issue, locations }) => { + const label = issue.severity === 0 ? "Error" : issue.severity === 3 ? "Hint" : "Warning"; + return [`[${label}] ${issue.message}`, ...locations].join("\n"); + }); +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index e30f67a..17d3e8a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -6,5 +6,6 @@ export { GltfInputBlock } from "./blocks/gltfInputBlock"; export { GltfOutputBlock, type GltfOutputBlockOptions } from "./blocks/gltfOutputBlock"; export { ObjInputBlock } from "./blocks/objInputBlock"; export { StlInputBlock } from "./blocks/stlInputBlock"; +export { ValidateBlock, type ValidateBlockOptions } from "./blocks/validateBlock"; export { NodeAsset } from "./nodeAsset"; export { NodeAssetContext } from "./nodeAssetContext"; diff --git a/packages/core/src/types/gltf-validator.d.ts b/packages/core/src/types/gltf-validator.d.ts new file mode 100644 index 0000000..cd3101d --- /dev/null +++ b/packages/core/src/types/gltf-validator.d.ts @@ -0,0 +1,30 @@ +declare module "gltf-validator" { + export interface ValidationIssue { + readonly code: string; + readonly message: string; + readonly severity: 0 | 1 | 2 | 3; + readonly pointer?: string; + readonly offset?: number; + } + + export interface ValidationReport { + readonly uri?: string; + readonly issues: { + readonly numErrors: number; + readonly numWarnings: number; + readonly numInfos: number; + readonly numHints: number; + readonly messages: readonly ValidationIssue[]; + readonly truncated: boolean; + }; + } + + export interface ValidationOptions { + readonly uri?: string; + readonly maxIssues?: number; + readonly ignoredIssues?: readonly string[]; + readonly externalResourceFunction?: (uri: string) => Promise; + } + + export function validateString(json: string, options?: ValidationOptions): Promise; +} diff --git a/packages/core/vite.config.ts b/packages/core/vite.config.ts index ff3a332..11e8308 100644 --- a/packages/core/vite.config.ts +++ b/packages/core/vite.config.ts @@ -20,7 +20,12 @@ export default defineConfig({ }, rollupOptions: { external: (id) => - /^@babylonjs\//.test(id) || /^@gltf-transform\//.test(id) || /^babylonpress-ktx2-encoder(?:\/|$)/.test(id) || /^meshoptimizer$/.test(id) || /^sharp$/.test(id), + /^@babylonjs\//.test(id) || + /^@gltf-transform\//.test(id) || + /^babylonpress-ktx2-encoder(?:\/|$)/.test(id) || + /^gltf-validator$/.test(id) || + /^meshoptimizer$/.test(id) || + /^sharp$/.test(id), }, }, plugins: [ diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5fb5133..cd55e14 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -35,6 +35,9 @@ importers: globals: specifier: ^17.7.0 version: 17.11.0 + gltf-validator: + specifier: 2.0.0-dev.3.10 + version: 2.0.0-dev.3.10 prettier: specifier: ^3.9.6 version: 3.9.6 @@ -92,6 +95,9 @@ importers: draco3dgltf: specifier: 1.5.7 version: 1.5.7 + gltf-validator: + specifier: 2.0.0-dev.3.10 + version: 2.0.0-dev.3.10 meshoptimizer: specifier: 1.2.0 version: 1.2.0 @@ -1175,6 +1181,9 @@ packages: resolution: {integrity: sha512-Z2I8hM+PbJDXQDq3Icgpzv+mPdwr68iZUU9d5WW4FuXfDUQfkZaZuvjMv42/5crNyw154+9+VWXbYrUgDXbxNw==} engines: {node: '>=18'} + gltf-validator@2.0.0-dev.3.10: + resolution: {integrity: sha512-odJ4k0tRkGXiDGn78yDBg+fBbAIvBnXxh3RwAta0emSxGtyagFE8B4xELB1oYe3S5RD8Ci3uZAsZaascH2LAEQ==} + graceful-fs@4.2.11: resolution: {integrity: sha512-RbJ5/jmFcNNCcDV5o9eTnBLJ/HszWV0P73bc+Ff4nS/rJj+YaS6IGyiOL0VoBYX+l1Wrl3k63h/KrH+nhJ0XvQ==} @@ -2629,6 +2638,8 @@ snapshots: globals@17.11.0: {} + gltf-validator@2.0.0-dev.3.10: {} + graceful-fs@4.2.11: {} has-flag@4.0.0: {} diff --git a/tests/bundle/browserConsumerBundle.test.ts b/tests/bundle/browserConsumerBundle.test.ts index 6bd29d1..1376f89 100644 --- a/tests/bundle/browserConsumerBundle.test.ts +++ b/tests/bundle/browserConsumerBundle.test.ts @@ -48,14 +48,16 @@ describe("browser consumer bundle", () => { ); try { - const { EncodeDracoBlock, GltfInputBlock, GltfOutputBlock, NodeAsset } = (await import( + const { EncodeDracoBlock, GltfInputBlock, GltfOutputBlock, NodeAsset, ValidateBlock } = (await import( `${pathToFileURL(PublishedEntryPath).href}?test=${Date.now()}` )) as typeof NodeAssets; const source = new GltfInputBlock({ input: url }); const encoder = new EncodeDracoBlock(); + const validate = new ValidateBlock(); const destination = new GltfOutputBlock(); source.output.connectTo(encoder.input); - encoder.output.connectTo(destination.input); + encoder.output.connectTo(validate.input); + validate.output.connectTo(destination.input); await parseGlbAsync(await new NodeAsset({ name: "published-node-entry", outputBlock: destination }).executeAsync()); } finally { diff --git a/tests/e2e/cli.test.ts b/tests/e2e/cli.test.ts index 8b2fa98..d090722 100644 --- a/tests/e2e/cli.test.ts +++ b/tests/e2e/cli.test.ts @@ -38,7 +38,7 @@ describe("Node Assets CLI", () => { const result = await runNodeAsync([launcher, ...args], directory); expect(result.code).toBe(0); expect(result.stderr).toBe(""); - for (const term of ["pipeline", ".gltf", ".glb", "draco", "meshopt", "ktx2", "--stats", "--benchmark"]) { + for (const term of ["pipeline", ".gltf", ".glb", "draco", "meshopt", "ktx2", "validate", "--stats", "--benchmark"]) { expect(result.stdout).toContain(term); } }); @@ -237,9 +237,13 @@ describe("Node Assets CLI", () => { }); it.each([ + { blocks: ["validate"], geometry: undefined, ktx2: false }, { blocks: ["draco"], geometry: "KHR_draco_mesh_compression", ktx2: false }, { blocks: ["meshopt"], geometry: "EXT_meshopt_compression", ktx2: false }, { blocks: ["ktx2"], geometry: undefined, ktx2: true }, + { blocks: ["validate", "draco", "validate"], geometry: "KHR_draco_mesh_compression", ktx2: false }, + { blocks: ["meshopt", "validate"], geometry: "EXT_meshopt_compression", ktx2: false }, + { blocks: ["ktx2", "validate"], geometry: undefined, ktx2: true }, { blocks: ["ktx2", "draco"], geometry: "KHR_draco_mesh_compression", ktx2: true }, { blocks: ["ktx2", "meshopt"], geometry: "EXT_meshopt_compression", ktx2: true }, ])( @@ -265,6 +269,46 @@ describe("Node Assets CLI", () => { 60_000 ); + it("prints validation diagnostics and writes output for a valid document", async () => { + const source = join(directory, "validate-warnings.gltf"); + const output = join(directory, "validate-warnings.glb"); + const io = new NodeIO(); + const document = await io.read(input); + document.createNode(); + await io.write(source, document); + + const result = await runNodeAsync([launcher, "pipeline", source, "validate", output], directory); + + expect(result.code).toBe(0); + expect(result.stdout).toContain(`\u2705 ${source} is valid`); + expect(result.stdout).toContain("[Warning]"); + expect(result.stdout).toContain("at /nodes/1"); + expect((await readGlbAsync(output)).json.meshes).toHaveLength(1); + }); + + it("fails validation without creating output or printing success reports", async () => { + const source = join(directory, "validate-invalid.gltf"); + const output = join(directory, "validate-invalid.glb"); + const io = new NodeIO(); + const document = await io.read(input); + document + .getRoot() + .listAccessors()[2]! + .setArray(new Uint16Array([0, 1, 99])); + await io.write(source, document); + + const result = await runNodeAsync([launcher, "pipeline", source, "validate", output, "--stats", "--benchmark"], directory); + + expect(result.code).toBe(1); + expect(result.stdout).toContain("[Error]"); + expect(result.stdout).not.toContain("is valid"); + expect(result.stdout).not.toContain("Wrote "); + expect(result.stdout).not.toContain("Stats:"); + expect(result.stdout).not.toContain("Benchmark:"); + expect(result.stderr.trim()).not.toBe(""); + await expect(readFile(output)).rejects.toThrow(); + }); + it.each([ { blocks: ["draco", "draco"], first: EncodeDracoBlock, second: EncodeDracoBlock }, { blocks: ["ktx2", "ktx2"], first: EncodeKTX2Block, second: EncodeKTX2Block }, diff --git a/tests/integration/validate.test.ts b/tests/integration/validate.test.ts new file mode 100644 index 0000000..c88b792 --- /dev/null +++ b/tests/integration/validate.test.ts @@ -0,0 +1,130 @@ +import { NodeIO } from "@gltf-transform/core"; +import { KHRMaterialsDiffuseTransmission, KHRMaterialsIOR } from "@gltf-transform/extensions"; +import { validateString } from "gltf-validator"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { GltfInputBlock, GltfOutputBlock, NodeAsset, ValidateBlock } from "../../packages/core/src/index"; +import { parseGlbAsync } from "../helpers/glb"; +import { generateGltfJson, generateTexturedGltfJson } from "../helpers/gltf"; + +afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); +}); + +describe("document validation", () => { + it("returns the same valid document with its content unchanged", async () => { + const io = new NodeIO(); + const document = await io.readJSON({ json: JSON.parse(generateGltfJson()), resources: {} }); + const before = await io.writeJSON(document); + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + + const result = await new NodeAsset({ name: "valid-document", outputBlock: new ValidateBlock({ input: document }) }).executeAsync(); + + expect(result).toBe(document); + expect(await io.writeJSON(document)).toEqual(before); + expect(log.mock.calls.flat()).toEqual(["\u2705 scene.glb is valid"]); + }); + + it("connects between document input and GLB output blocks", async () => { + vi.spyOn(console, "log").mockImplementation(() => {}); + vi.stubGlobal( + "fetch", + vi.fn(() => Promise.resolve(new Response(generateGltfJson()))) + ); + const source = new GltfInputBlock({ input: "https://example.com/model.gltf" }); + const validate = new ValidateBlock(); + const destination = new GltfOutputBlock(); + source.output.connectTo(validate.input); + validate.output.connectTo(destination.input); + + const parsed = await parseGlbAsync(await new NodeAsset({ name: "validated-pipeline", outputBlock: destination }).executeAsync()); + + expect(parsed.json.meshes).toHaveLength(1); + }); + + it("fails on invalid binary accessor data, not just invalid JSON", async () => { + const document = await new NodeIO().readJSON({ json: JSON.parse(generateGltfJson()), resources: {} }); + document + .getRoot() + .listAccessors()[2]! + .setArray(new Uint16Array([0, 1, 99])); + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + + await expect(new NodeAsset({ name: "invalid-indices", outputBlock: new ValidateBlock({ input: document }) }).executeAsync()).rejects.toThrow(); + + expect(log.mock.calls.flat().join("\n")).toContain("[Error]"); + expect(log.mock.calls.flat().join("\n")).not.toContain("is valid"); + }); + + it("checks image bytes from serialized resources", async () => { + const document = await new NodeIO().readJSON({ json: JSON.parse(generateTexturedGltfJson()), resources: {} }); + const texture = document.getRoot().listTextures()[0]!; + texture.setImage(texture.getImage()!.slice(0, 16)); + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + + await expect(new NodeAsset({ name: "invalid-image", outputBlock: new ValidateBlock({ input: document }) }).executeAsync()).rejects.toThrow(); + + expect(log.mock.calls.flat().join("\n")).toContain("[Error]"); + }); + + it("accepts multiple buffers and encoded resource URIs without fetching them", async () => { + const document = await new NodeIO().readJSON({ json: JSON.parse(generateTexturedGltfJson()), resources: {} }); + document.getRoot().listBuffers()[0]!.setURI("mesh%20data.bin"); + const indices = document.createBuffer().setURI("indices.bin"); + document.getRoot().listAccessors()[3]!.setBuffer(indices); + document.getRoot().listTextures()[0]!.setURI("texture%20(1).png"); + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + + expect(await new NodeAsset({ name: "resource-uris", outputBlock: new ValidateBlock({ input: document }) }).executeAsync()).toBe(document); + expect(log.mock.calls.flat().join("\n")).not.toContain("[Error]"); + }); + + it("ignores unsupported extensions without removing them from the document", async () => { + const document = await new NodeIO().readJSON({ json: JSON.parse(generateGltfJson()), resources: {} }); + const extension = document.createExtension(KHRMaterialsDiffuseTransmission); + const material = document.createMaterial().setExtension(extension.extensionName, extension.createDiffuseTransmission()); + document.getRoot().listMeshes()[0]!.listPrimitives()[0]!.setMaterial(material); + const serialized = await new NodeIO().registerExtensions([KHRMaterialsDiffuseTransmission]).writeJSON(document); + const baseline = await validateString(JSON.stringify(serialized.json)); + expect(baseline.issues.messages.some(({ code }) => code === "UNSUPPORTED_EXTENSION")).toBe(true); + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + + const result = await new NodeAsset({ name: "unsupported-extension", outputBlock: new ValidateBlock({ input: document }) }).executeAsync(); + + expect(result).toBe(document); + expect(result.hasExtension(extension.extensionName)).toBe(true); + expect(log.mock.calls.flat()).toEqual(["\u2705 scene.glb is valid"]); + }); + + it("validates supported extensions on a directly supplied document", async () => { + const document = await new NodeIO().readJSON({ json: JSON.parse(generateGltfJson()), resources: {} }); + const extension = document.createExtension(KHRMaterialsIOR); + const material = document.createMaterial().setExtension(extension.extensionName, extension.createIOR().setIOR(0.5)); + document.getRoot().listMeshes()[0]!.listPrimitives()[0]!.setMaterial(material); + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + + await expect(new NodeAsset({ name: "invalid-extension", outputBlock: new ValidateBlock({ input: document }) }).executeAsync()).rejects.toThrow(); + + expect(log.mock.calls.flat().join("\n")).toContain("[Error]"); + }); + + it("does not let a large number of informational issues hide a later data error", async () => { + const document = await new NodeIO().readJSON({ json: JSON.parse(generateGltfJson()), resources: {} }); + for (let i = 0; i < 10_001; i++) { + document.createNode(); + } + document + .getRoot() + .listAccessors()[2]! + .setArray(new Uint16Array([0, 1, 99])); + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + + await expect(new NodeAsset({ name: "many-issues", outputBlock: new ValidateBlock({ input: document }) }).executeAsync()).rejects.toThrow(); + + const output = log.mock.calls.flat().join("\n"); + expect(output).toContain("[Error]"); + expect(output).toContain("at /nodes/10001"); + expect(output).not.toContain("is valid"); + }); +}); diff --git a/tests/unit/validateBlock.test.ts b/tests/unit/validateBlock.test.ts new file mode 100644 index 0000000..d7c80b2 --- /dev/null +++ b/tests/unit/validateBlock.test.ts @@ -0,0 +1,110 @@ +import { Document } from "@gltf-transform/core"; +import { validateString, type ValidationIssue } from "gltf-validator"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { NodeAsset, ValidateBlock } from "../../packages/core/src/index"; + +vi.mock("gltf-validator", () => ({ validateString: vi.fn() })); + +afterEach(() => { + vi.restoreAllMocks(); + vi.resetAllMocks(); +}); + +describe("ValidateBlock diagnostics", () => { + it("prints the success label and groups the example's informational issues and hints", async () => { + const document = new Document(); + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + const messages: ValidationIssue[] = [ + { + code: "BUFFER_VIEW_TARGET_MISSING", + message: "bufferView.target should be set for vertex or index data.", + severity: 3, + pointer: "/meshes/0/primitives/0/attributes/POSITION", + }, + { + code: "BUFFER_VIEW_TARGET_MISSING", + message: "bufferView.target should be set for vertex or index data.", + severity: 3, + pointer: "/meshes/0/primitives/0/attributes/NORMAL", + }, + { + code: "BUFFER_VIEW_TARGET_MISSING", + message: "bufferView.target should be set for vertex or index data.", + severity: 3, + pointer: "/meshes/0/primitives/0/indices", + }, + { code: "NODE_EMPTY", message: "Empty node encountered.", severity: 2, pointer: "/nodes/1" }, + { code: "UNUSED_OBJECT", message: "This object may be unused.", severity: 2, pointer: "/bufferViews/2" }, + ]; + vi.mocked(validateString).mockResolvedValue({ + uri: "box%20(1).glb", + issues: { numErrors: 0, numWarnings: 0, numInfos: 2, numHints: 3, messages, truncated: false }, + }); + const block = new ValidateBlock({ input: document, uri: "box%20(1).glb" }); + + expect(await new NodeAsset({ name: "validation-report", outputBlock: block }).executeAsync()).toBe(document); + expect(log.mock.calls.flat().join("\n")).toBe( + [ + "\u2705 box%20(1).glb is valid", + "[Warning] This object may be unused.\n at /bufferViews/2", + "[Warning] Empty node encountered.\n at /nodes/1", + "[Hint] bufferView.target should be set for vertex or index data.\n at /meshes/0/primitives/0/indices\n at /meshes/0/primitives/0/attributes/NORMAL\n at /meshes/0/primitives/0/attributes/POSITION", + ].join("\n\n") + ); + }); + + it("logs warnings, including diagnostics at the document root", async () => { + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + vi.mocked(validateString).mockResolvedValue({ + issues: { + numErrors: 0, + numWarnings: 1, + numInfos: 0, + numHints: 0, + messages: [{ code: "TEST_WARNING", message: "Review this document.", severity: 1, pointer: "" }], + truncated: false, + }, + }); + + await new NodeAsset({ name: "warning", outputBlock: new ValidateBlock({ input: new Document() }) }).executeAsync(); + + expect(log.mock.calls.flat().join("\n")).toBe("\u2705 scene.glb is valid\n\n[Warning] Review this document.\n at /"); + }); + + it("logs every severity before rejecting a report containing errors", async () => { + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + vi.mocked(validateString).mockResolvedValue({ + issues: { + numErrors: 1, + numWarnings: 1, + numInfos: 1, + numHints: 1, + messages: [ + { code: "TEST_HINT", message: "A hint.", severity: 3, pointer: "/meshes/0" }, + { code: "TEST_INFO", message: "Information.", severity: 2, pointer: "/nodes/0" }, + { code: "TEST_WARNING", message: "A warning.", severity: 1, pointer: "/materials/0" }, + { code: "TEST_ERROR", message: "An error.", severity: 0, pointer: "/accessors/0" }, + ], + truncated: false, + }, + }); + + await expect(new NodeAsset({ name: "invalid", outputBlock: new ValidateBlock({ input: new Document() }) }).executeAsync()).rejects.toThrow(); + + expect(log.mock.calls.flat().join("\n")).toBe( + ["[Error] An error.\n at /accessors/0", "[Warning] A warning.\n at /materials/0", "[Warning] Information.\n at /nodes/0", "[Hint] A hint.\n at /meshes/0"].join( + "\n\n" + ) + ); + }); + + it("propagates validator failures without claiming success", async () => { + const log = vi.spyOn(console, "log").mockImplementation(() => {}); + vi.mocked(validateString).mockRejectedValue(new Error("Validator failed")); + + await expect(new NodeAsset({ name: "validator-failure", outputBlock: new ValidateBlock({ input: new Document() }) }).executeAsync()).rejects.toThrow(); + + expect(log.mock.calls).toEqual([]); + }); +}); From 96be7fad54f38fb48d5202b469d1b54874ddebec Mon Sep 17 00:00:00 2001 From: "Alex C. Huber" <91097647+alexchuber@users.noreply.github.com> Date: Thu, 17 Sep 2026 18:20:38 -0400 Subject: [PATCH 2/5] fix: decouple validation tests and reduce copies Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- package.json | 1 - packages/core/src/blocks/validateBlock.ts | 25 ++--- pnpm-lock.yaml | 3 - tests/integration/validate.test.ts | 100 ++++++++++++++++---- tests/unit/validateBlock.test.ts | 110 ---------------------- 5 files changed, 94 insertions(+), 145 deletions(-) delete mode 100644 tests/unit/validateBlock.test.ts diff --git a/package.json b/package.json index 9cb1b20..59c066f 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,6 @@ "eslint-config-prettier": "^10.1.8", "eslint-plugin-prettier": "^5.5.6", "globals": "^17.7.0", - "gltf-validator": "2.0.0-dev.3.10", "prettier": "^3.9.6", "typedoc": "^0.28.20", "typescript": "^6.0.3", diff --git a/packages/core/src/blocks/validateBlock.ts b/packages/core/src/blocks/validateBlock.ts index 8543020..0696cfc 100644 --- a/packages/core/src/blocks/validateBlock.ts +++ b/packages/core/src/blocks/validateBlock.ts @@ -33,11 +33,15 @@ const ValidateBlockDefinition = /* @__PURE__ */ defineBlock({ }, }); - const diagnostics = formatIssues(report.issues.messages); + let separator = ""; if (report.issues.numErrors === 0) { - diagnostics.unshift(`\u2705 ${uri} is valid`); + console.log(`\u2705 ${uri} is valid`); + separator = "\n"; + } + for (const diagnostic of formatIssues(report.issues.messages)) { + console.log(separator + diagnostic); + separator = "\n"; } - console.log(diagnostics.join("\n\n")); if (report.issues.numErrors > 0) { throw new Error(`glTF validation failed for "${uri}" with ${report.issues.numErrors} error(s).`); @@ -56,9 +60,10 @@ export class ValidateBlock extends Block { } } -function formatIssues(issues: readonly ValidationIssue[]): string[] { +function* formatIssues(issues: readonly ValidationIssue[]): Generator { const groups = new Map(); - for (const issue of [...issues].reverse()) { + for (let index = issues.length - 1; index >= 0; index--) { + const issue = issues[index]!; const key = JSON.stringify([issue.severity, issue.code, issue.message]); let group = groups.get(key); if (group === undefined) { @@ -71,10 +76,8 @@ function formatIssues(issues: readonly ValidationIssue[]): string[] { group.locations.push(` at byte ${issue.offset}`); } } - return [...groups.values()] - .sort((a, b) => a.issue.severity - b.issue.severity) - .map(({ issue, locations }) => { - const label = issue.severity === 0 ? "Error" : issue.severity === 3 ? "Hint" : "Warning"; - return [`[${label}] ${issue.message}`, ...locations].join("\n"); - }); + for (const { issue, locations } of [...groups.values()].sort((a, b) => a.issue.severity - b.issue.severity)) { + const label = issue.severity === 0 ? "Error" : issue.severity === 3 ? "Hint" : "Warning"; + yield `[${label}] ${issue.message}${locations.length > 0 ? "\n" + locations.join("\n") : ""}`; + } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index cd55e14..4398bc6 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -35,9 +35,6 @@ importers: globals: specifier: ^17.7.0 version: 17.11.0 - gltf-validator: - specifier: 2.0.0-dev.3.10 - version: 2.0.0-dev.3.10 prettier: specifier: ^3.9.6 version: 3.9.6 diff --git a/tests/integration/validate.test.ts b/tests/integration/validate.test.ts index c88b792..c1e8089 100644 --- a/tests/integration/validate.test.ts +++ b/tests/integration/validate.test.ts @@ -1,6 +1,7 @@ +import { format } from "node:util"; + import { NodeIO } from "@gltf-transform/core"; import { KHRMaterialsDiffuseTransmission, KHRMaterialsIOR } from "@gltf-transform/extensions"; -import { validateString } from "gltf-validator"; import { afterEach, describe, expect, it, vi } from "vitest"; import { GltfInputBlock, GltfOutputBlock, NodeAsset, ValidateBlock } from "../../packages/core/src/index"; @@ -17,17 +18,71 @@ describe("document validation", () => { const io = new NodeIO(); const document = await io.readJSON({ json: JSON.parse(generateGltfJson()), resources: {} }); const before = await io.writeJSON(document); - const log = vi.spyOn(console, "log").mockImplementation(() => {}); + const readOutput = captureOutput(); const result = await new NodeAsset({ name: "valid-document", outputBlock: new ValidateBlock({ input: document }) }).executeAsync(); expect(result).toBe(document); expect(await io.writeJSON(document)).toEqual(before); - expect(log.mock.calls.flat()).toEqual(["\u2705 scene.glb is valid"]); + expect(readOutput()).toBe("\u2705 scene.glb is valid"); + }); + + it("prints the URI label and groups informational issues by message and location", async () => { + const document = await new NodeIO().readJSON({ json: JSON.parse(generateGltfJson()), resources: {} }); + document.createNode(); + document.createNode(); + const readOutput = captureOutput(); + + const result = await new NodeAsset({ name: "grouped-diagnostics", outputBlock: new ValidateBlock({ input: document, uri: "box%20(1).glb" }) }).executeAsync(); + + expect(result).toBe(document); + expect(readOutput()).toBe( + [ + "\u2705 box%20(1).glb is valid", + "[Warning] This object may be unused.\n at /nodes/2\n at /nodes/1", + "[Warning] Empty node encountered.\n at /nodes/2\n at /nodes/1", + ].join("\n\n") + ); + }); + + it("succeeds with warnings from the validator", async () => { + const document = await new NodeIO().readJSON({ json: JSON.parse(generateTexturedGltfJson()), resources: {} }); + document + .getRoot() + .listTextures()[0]! + .setImage(new Uint8Array([0, 1, 2, 3])); + const readOutput = captureOutput(); + + const result = await new NodeAsset({ name: "warning", outputBlock: new ValidateBlock({ input: document }) }).executeAsync(); + + expect(result).toBe(document); + expect(readOutput()).toBe("\u2705 scene.glb is valid\n\n[Warning] Image format not recognized.\n at /images/0"); + }); + + it("logs warnings and informational issues even when validation fails", async () => { + const document = await new NodeIO().readJSON({ json: JSON.parse(generateTexturedGltfJson()), resources: {} }); + document + .getRoot() + .listTextures()[0]! + .setImage(new Uint8Array([0, 1, 2, 3])); + document + .getRoot() + .listAccessors()[3]! + .setArray(new Uint16Array([0, 1, 99])); + document.createNode(); + const readOutput = captureOutput(); + + await expect(new NodeAsset({ name: "errors-and-warnings", outputBlock: new ValidateBlock({ input: document }) }).executeAsync()).rejects.toThrow(); + + const output = readOutput(); + expect(output).toMatch(/^\[Error\]/); + expect(output).toContain("\n\n[Warning] Image format not recognized.\n at /images/0"); + expect(output).toContain("\n\n[Warning] Empty node encountered.\n at /nodes/1"); + expect(output).not.toContain("is valid"); }); it("connects between document input and GLB output blocks", async () => { - vi.spyOn(console, "log").mockImplementation(() => {}); + captureOutput(); vi.stubGlobal( "fetch", vi.fn(() => Promise.resolve(new Response(generateGltfJson()))) @@ -49,23 +104,23 @@ describe("document validation", () => { .getRoot() .listAccessors()[2]! .setArray(new Uint16Array([0, 1, 99])); - const log = vi.spyOn(console, "log").mockImplementation(() => {}); + const readOutput = captureOutput(); await expect(new NodeAsset({ name: "invalid-indices", outputBlock: new ValidateBlock({ input: document }) }).executeAsync()).rejects.toThrow(); - expect(log.mock.calls.flat().join("\n")).toContain("[Error]"); - expect(log.mock.calls.flat().join("\n")).not.toContain("is valid"); + expect(readOutput()).toContain("[Error]"); + expect(readOutput()).not.toContain("is valid"); }); it("checks image bytes from serialized resources", async () => { const document = await new NodeIO().readJSON({ json: JSON.parse(generateTexturedGltfJson()), resources: {} }); const texture = document.getRoot().listTextures()[0]!; texture.setImage(texture.getImage()!.slice(0, 16)); - const log = vi.spyOn(console, "log").mockImplementation(() => {}); + const readOutput = captureOutput(); await expect(new NodeAsset({ name: "invalid-image", outputBlock: new ValidateBlock({ input: document }) }).executeAsync()).rejects.toThrow(); - expect(log.mock.calls.flat().join("\n")).toContain("[Error]"); + expect(readOutput()).toContain("[Error]"); }); it("accepts multiple buffers and encoded resource URIs without fetching them", async () => { @@ -74,10 +129,10 @@ describe("document validation", () => { const indices = document.createBuffer().setURI("indices.bin"); document.getRoot().listAccessors()[3]!.setBuffer(indices); document.getRoot().listTextures()[0]!.setURI("texture%20(1).png"); - const log = vi.spyOn(console, "log").mockImplementation(() => {}); + const readOutput = captureOutput(); expect(await new NodeAsset({ name: "resource-uris", outputBlock: new ValidateBlock({ input: document }) }).executeAsync()).toBe(document); - expect(log.mock.calls.flat().join("\n")).not.toContain("[Error]"); + expect(readOutput()).not.toContain("[Error]"); }); it("ignores unsupported extensions without removing them from the document", async () => { @@ -85,16 +140,13 @@ describe("document validation", () => { const extension = document.createExtension(KHRMaterialsDiffuseTransmission); const material = document.createMaterial().setExtension(extension.extensionName, extension.createDiffuseTransmission()); document.getRoot().listMeshes()[0]!.listPrimitives()[0]!.setMaterial(material); - const serialized = await new NodeIO().registerExtensions([KHRMaterialsDiffuseTransmission]).writeJSON(document); - const baseline = await validateString(JSON.stringify(serialized.json)); - expect(baseline.issues.messages.some(({ code }) => code === "UNSUPPORTED_EXTENSION")).toBe(true); - const log = vi.spyOn(console, "log").mockImplementation(() => {}); + const readOutput = captureOutput(); const result = await new NodeAsset({ name: "unsupported-extension", outputBlock: new ValidateBlock({ input: document }) }).executeAsync(); expect(result).toBe(document); expect(result.hasExtension(extension.extensionName)).toBe(true); - expect(log.mock.calls.flat()).toEqual(["\u2705 scene.glb is valid"]); + expect(readOutput()).toBe("\u2705 scene.glb is valid"); }); it("validates supported extensions on a directly supplied document", async () => { @@ -102,11 +154,11 @@ describe("document validation", () => { const extension = document.createExtension(KHRMaterialsIOR); const material = document.createMaterial().setExtension(extension.extensionName, extension.createIOR().setIOR(0.5)); document.getRoot().listMeshes()[0]!.listPrimitives()[0]!.setMaterial(material); - const log = vi.spyOn(console, "log").mockImplementation(() => {}); + const readOutput = captureOutput(); await expect(new NodeAsset({ name: "invalid-extension", outputBlock: new ValidateBlock({ input: document }) }).executeAsync()).rejects.toThrow(); - expect(log.mock.calls.flat().join("\n")).toContain("[Error]"); + expect(readOutput()).toContain("[Error]"); }); it("does not let a large number of informational issues hide a later data error", async () => { @@ -118,13 +170,21 @@ describe("document validation", () => { .getRoot() .listAccessors()[2]! .setArray(new Uint16Array([0, 1, 99])); - const log = vi.spyOn(console, "log").mockImplementation(() => {}); + const readOutput = captureOutput(); await expect(new NodeAsset({ name: "many-issues", outputBlock: new ValidateBlock({ input: document }) }).executeAsync()).rejects.toThrow(); - const output = log.mock.calls.flat().join("\n"); + const output = readOutput(); expect(output).toContain("[Error]"); expect(output).toContain("at /nodes/10001"); expect(output).not.toContain("is valid"); }); + + function captureOutput(): () => string { + const lines: string[] = []; + vi.spyOn(console, "log").mockImplementation((...args: unknown[]) => { + lines.push(format(...args)); + }); + return () => lines.join("\n"); + } }); diff --git a/tests/unit/validateBlock.test.ts b/tests/unit/validateBlock.test.ts deleted file mode 100644 index d7c80b2..0000000 --- a/tests/unit/validateBlock.test.ts +++ /dev/null @@ -1,110 +0,0 @@ -import { Document } from "@gltf-transform/core"; -import { validateString, type ValidationIssue } from "gltf-validator"; -import { afterEach, describe, expect, it, vi } from "vitest"; - -import { NodeAsset, ValidateBlock } from "../../packages/core/src/index"; - -vi.mock("gltf-validator", () => ({ validateString: vi.fn() })); - -afterEach(() => { - vi.restoreAllMocks(); - vi.resetAllMocks(); -}); - -describe("ValidateBlock diagnostics", () => { - it("prints the success label and groups the example's informational issues and hints", async () => { - const document = new Document(); - const log = vi.spyOn(console, "log").mockImplementation(() => {}); - const messages: ValidationIssue[] = [ - { - code: "BUFFER_VIEW_TARGET_MISSING", - message: "bufferView.target should be set for vertex or index data.", - severity: 3, - pointer: "/meshes/0/primitives/0/attributes/POSITION", - }, - { - code: "BUFFER_VIEW_TARGET_MISSING", - message: "bufferView.target should be set for vertex or index data.", - severity: 3, - pointer: "/meshes/0/primitives/0/attributes/NORMAL", - }, - { - code: "BUFFER_VIEW_TARGET_MISSING", - message: "bufferView.target should be set for vertex or index data.", - severity: 3, - pointer: "/meshes/0/primitives/0/indices", - }, - { code: "NODE_EMPTY", message: "Empty node encountered.", severity: 2, pointer: "/nodes/1" }, - { code: "UNUSED_OBJECT", message: "This object may be unused.", severity: 2, pointer: "/bufferViews/2" }, - ]; - vi.mocked(validateString).mockResolvedValue({ - uri: "box%20(1).glb", - issues: { numErrors: 0, numWarnings: 0, numInfos: 2, numHints: 3, messages, truncated: false }, - }); - const block = new ValidateBlock({ input: document, uri: "box%20(1).glb" }); - - expect(await new NodeAsset({ name: "validation-report", outputBlock: block }).executeAsync()).toBe(document); - expect(log.mock.calls.flat().join("\n")).toBe( - [ - "\u2705 box%20(1).glb is valid", - "[Warning] This object may be unused.\n at /bufferViews/2", - "[Warning] Empty node encountered.\n at /nodes/1", - "[Hint] bufferView.target should be set for vertex or index data.\n at /meshes/0/primitives/0/indices\n at /meshes/0/primitives/0/attributes/NORMAL\n at /meshes/0/primitives/0/attributes/POSITION", - ].join("\n\n") - ); - }); - - it("logs warnings, including diagnostics at the document root", async () => { - const log = vi.spyOn(console, "log").mockImplementation(() => {}); - vi.mocked(validateString).mockResolvedValue({ - issues: { - numErrors: 0, - numWarnings: 1, - numInfos: 0, - numHints: 0, - messages: [{ code: "TEST_WARNING", message: "Review this document.", severity: 1, pointer: "" }], - truncated: false, - }, - }); - - await new NodeAsset({ name: "warning", outputBlock: new ValidateBlock({ input: new Document() }) }).executeAsync(); - - expect(log.mock.calls.flat().join("\n")).toBe("\u2705 scene.glb is valid\n\n[Warning] Review this document.\n at /"); - }); - - it("logs every severity before rejecting a report containing errors", async () => { - const log = vi.spyOn(console, "log").mockImplementation(() => {}); - vi.mocked(validateString).mockResolvedValue({ - issues: { - numErrors: 1, - numWarnings: 1, - numInfos: 1, - numHints: 1, - messages: [ - { code: "TEST_HINT", message: "A hint.", severity: 3, pointer: "/meshes/0" }, - { code: "TEST_INFO", message: "Information.", severity: 2, pointer: "/nodes/0" }, - { code: "TEST_WARNING", message: "A warning.", severity: 1, pointer: "/materials/0" }, - { code: "TEST_ERROR", message: "An error.", severity: 0, pointer: "/accessors/0" }, - ], - truncated: false, - }, - }); - - await expect(new NodeAsset({ name: "invalid", outputBlock: new ValidateBlock({ input: new Document() }) }).executeAsync()).rejects.toThrow(); - - expect(log.mock.calls.flat().join("\n")).toBe( - ["[Error] An error.\n at /accessors/0", "[Warning] A warning.\n at /materials/0", "[Warning] Information.\n at /nodes/0", "[Hint] A hint.\n at /meshes/0"].join( - "\n\n" - ) - ); - }); - - it("propagates validator failures without claiming success", async () => { - const log = vi.spyOn(console, "log").mockImplementation(() => {}); - vi.mocked(validateString).mockRejectedValue(new Error("Validator failed")); - - await expect(new NodeAsset({ name: "validator-failure", outputBlock: new ValidateBlock({ input: new Document() }) }).executeAsync()).rejects.toThrow(); - - expect(log.mock.calls).toEqual([]); - }); -}); From 115106a6a80cccb602dcd631785fa7d5357b5904 Mon Sep 17 00:00:00 2001 From: "Alex C. Huber" <91097647+alexchuber@users.noreply.github.com> Date: Fri, 18 Sep 2026 17:22:51 -0400 Subject: [PATCH 3/5] docs: trim validation documentation Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- packages/cli/README.md | 7 ------- packages/core/docs/blocks.md | 2 +- packages/core/docs/usage.md | 40 ------------------------------------ 3 files changed, 1 insertion(+), 48 deletions(-) diff --git a/packages/cli/README.md b/packages/cli/README.md index ccc69f5..141db3c 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -30,13 +30,6 @@ node-assets pipeline [operation...] [--stats] [--benchmark] Without specifying operations, the CLI reads the input and writes it back out as the target output format. -`validate` checks the current document, including buffers and images, with the -Khronos glTF Validator. It prints grouped diagnostics using the input path as the -label. Warnings, informational issues, and hints do not fail the run; -`UNSUPPORTED_EXTENSION` is ignored. Any error stops the pipeline with exit code 1 -before writing output. Place `validate` before or after other operations to check -the document at that point; it does not validate the original file's packaging. - Run `node-assets --help` for command help or `node-assets --version` for the installed version. diff --git a/packages/core/docs/blocks.md b/packages/core/docs/blocks.md index 88241a4..1d3f179 100644 --- a/packages/core/docs/blocks.md +++ b/packages/core/docs/blocks.md @@ -30,7 +30,7 @@ - Input: `Document` - Output: the same `Document` - Uses: `gltf-validator`; execution-scoped `PlatformIO` - - Behavior: Serializes the current document and validates its JSON, buffers, and images. Logs grouped errors, warnings (including informational issues), and hints with their locations. Throws on any errors; otherwise prints a success message. Ignores `UNSUPPORTED_EXTENSION` and does not truncate diagnostics. + - Behavior: Validates the document, throwing on errors and logging other issues. Ignores `UNSUPPORTED_EXTENSION`. - Options: `uri` sets the diagnostic label (default: `scene.glb`); it is not loaded. - `EncodeKTX2Block` - Input: `Document` diff --git a/packages/core/docs/usage.md b/packages/core/docs/usage.md index c3c2d05..d154f5c 100644 --- a/packages/core/docs/usage.md +++ b/packages/core/docs/usage.md @@ -16,46 +16,6 @@ const asset = new NodeAsset({ const result = await asset.executeAsync(); ``` -# Example: Validating a document - -Insert a `ValidateBlock` wherever the current document should be checked by the -Khronos glTF Validator, or use it as the output block to return the validated -`Document`. - -```ts -const source = new GltfInputBlock({ input: "box%20(1).glb" }); -const validate = new ValidateBlock({ uri: "box%20(1).glb" }); -const destination = new GltfOutputBlock(); - -source.output.connectTo(validate.input); -validate.output.connectTo(destination.input); - -const asset = new NodeAsset({ name: "validated-gltf", outputBlock: destination }); -const result = await asset.executeAsync(); -``` - -Validation checks a serialized copy of the current document, including its -buffers and images, rather than the original source file. It returns the same -`Document` reference on success. The optional `uri` is a display label, not a file -to load; it defaults to `scene.glb` because documents do not retain source paths. - -If there are no errors, the block prints a check mark followed by ` is valid` -using `console.log`. Diagnostics are grouped by severity, code, and message, with -each location on an indented `at` line. Errors are labeled `[Error]`, warnings and -informational issues are labeled `[Warning]`, and hints are labeled `[Hint]`. -Errors come first, then warnings, informational issues, and hints. All reported -diagnostics are logged, even when validation fails. Any validation error rejects -the execution, preventing downstream blocks from running. - -`UNSUPPORTED_EXTENSION` issues are ignored. Other diagnostics are not truncated, -so earlier warnings cannot hide a later error. - -The CLI supports the same block: - -```sh -node-assets pipeline input.glb validate output.glb -``` - # Example: CLI run reports ```sh From 7cf58cdffee2323f5b1b76f022c71b1a52753c72 Mon Sep 17 00:00:00 2001 From: "Alex C. Huber" <91097647+alexchuber@users.noreply.github.com> Date: Fri, 18 Sep 2026 17:25:08 -0400 Subject: [PATCH 4/5] docs: simplify CLI validation wording Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- packages/cli/README.md | 1 - packages/cli/src/pipeline.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/cli/README.md b/packages/cli/README.md index 141db3c..e509a71 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -13,7 +13,6 @@ A pipeline is a sequence of operations applied to a 3D asset. The CLI allows you ```sh node-assets pipeline input.gltf output.glb node-assets pipeline input.glb ktx2 draco output.glb -node-assets pipeline input.glb validate output.glb ``` The command syntax is: diff --git a/packages/cli/src/pipeline.ts b/packages/cli/src/pipeline.ts index d61a887..aa3abbe 100644 --- a/packages/cli/src/pipeline.ts +++ b/packages/cli/src/pipeline.ts @@ -19,7 +19,7 @@ export function getPipelineDefinitions() { operations: [ { name: "validate", - description: "Validate the document and fail on errors", + description: "Validate the file and fail on errors", create: (library: typeof NodeAssets, path: string) => new library.ValidateBlock({ uri: path }), }, { From ec9c76320efc08ff96ece4ebe169e6dd4443fef6 Mon Sep 17 00:00:00 2001 From: "Alex C. Huber" <91097647+alexchuber@users.noreply.github.com> Date: Fri, 18 Sep 2026 17:27:32 -0400 Subject: [PATCH 5/5] refactor: remove validation label option Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- packages/cli/src/pipeline.ts | 4 ++-- packages/core/docs/blocks.md | 1 - packages/core/src/blocks/validateBlock.ts | 15 +++++---------- packages/core/src/types/gltf-validator.d.ts | 2 -- tests/e2e/cli.test.ts | 2 +- tests/integration/validate.test.ts | 18 ++++++++---------- 6 files changed, 16 insertions(+), 26 deletions(-) diff --git a/packages/cli/src/pipeline.ts b/packages/cli/src/pipeline.ts index aa3abbe..2d4bcbd 100644 --- a/packages/cli/src/pipeline.ts +++ b/packages/cli/src/pipeline.ts @@ -20,7 +20,7 @@ export function getPipelineDefinitions() { { name: "validate", description: "Validate the file and fail on errors", - create: (library: typeof NodeAssets, path: string) => new library.ValidateBlock({ uri: path }), + create: (library: typeof NodeAssets) => new library.ValidateBlock(), }, { name: "draco", @@ -71,7 +71,7 @@ export async function createPipelineAsync({ inputPath, outputPath, blockNames }: const source = inputDefinition.create(library, inputPath); let previous = source.output; for (const transform of transforms) { - const block = transform.create(library, inputPath); + const block = transform.create(library); previous.connectTo(block.input); previous = block.output; } diff --git a/packages/core/docs/blocks.md b/packages/core/docs/blocks.md index 1d3f179..4e538aa 100644 --- a/packages/core/docs/blocks.md +++ b/packages/core/docs/blocks.md @@ -31,7 +31,6 @@ - Output: the same `Document` - Uses: `gltf-validator`; execution-scoped `PlatformIO` - Behavior: Validates the document, throwing on errors and logging other issues. Ignores `UNSUPPORTED_EXTENSION`. - - Options: `uri` sets the diagnostic label (default: `scene.glb`); it is not loaded. - `EncodeKTX2Block` - Input: `Document` - Output: `Document` (but in future should be type that locks images and/or textures) diff --git a/packages/core/src/blocks/validateBlock.ts b/packages/core/src/blocks/validateBlock.ts index 0696cfc..9ae19c1 100644 --- a/packages/core/src/blocks/validateBlock.ts +++ b/packages/core/src/blocks/validateBlock.ts @@ -2,26 +2,21 @@ import { ALL_EXTENSIONS } from "@gltf-transform/extensions"; import type { ValidationIssue } from "gltf-validator"; import { GltfDocumentType } from "../connectionPoints/gltfDocument"; -import { UrlType } from "../connectionPoints/url"; import { PlatformIOResource } from "../resources/platformIOResource"; import { Block, type BlockOptions } from "./block"; -import { defineBlock, value } from "./blockDefinition"; +import { defineBlock } from "./blockDefinition"; const ValidateBlockDefinition = /* @__PURE__ */ defineBlock({ type: "transform.validate", input: GltfDocumentType, output: GltfDocumentType, - config: { - uri: /* @__PURE__ */ value(UrlType, "scene.glb"), - }, resources: { io: PlatformIOResource, }, - runAsync: async (document, { uri }, { io }) => { + runAsync: async (document, _config, { io }) => { const { validateString } = await import("gltf-validator"); const { json, resources } = await io.registerExtensions(ALL_EXTENSIONS).writeJSON(document); const report = await validateString(JSON.stringify(json), { - uri, ignoredIssues: ["UNSUPPORTED_EXTENSION"], maxIssues: 0, externalResourceFunction: async (resourceUri) => { @@ -35,7 +30,7 @@ const ValidateBlockDefinition = /* @__PURE__ */ defineBlock({ let separator = ""; if (report.issues.numErrors === 0) { - console.log(`\u2705 ${uri} is valid`); + console.log("\u2705 glTF is valid"); separator = "\n"; } for (const diagnostic of formatIssues(report.issues.messages)) { @@ -44,13 +39,13 @@ const ValidateBlockDefinition = /* @__PURE__ */ defineBlock({ } if (report.issues.numErrors > 0) { - throw new Error(`glTF validation failed for "${uri}" with ${report.issues.numErrors} error(s).`); + throw new Error(`glTF validation failed with ${report.issues.numErrors} error(s).`); } return document; }, }); -/** Options for the block, its input document, and a diagnostic URI label (default: scene.glb). */ +/** Options for naming the block or supplying its initial input. */ export type ValidateBlockOptions = BlockOptions; /** Validates a document, logs grouped diagnostics, and rejects execution on validation errors. */ diff --git a/packages/core/src/types/gltf-validator.d.ts b/packages/core/src/types/gltf-validator.d.ts index cd3101d..1a071cb 100644 --- a/packages/core/src/types/gltf-validator.d.ts +++ b/packages/core/src/types/gltf-validator.d.ts @@ -8,7 +8,6 @@ declare module "gltf-validator" { } export interface ValidationReport { - readonly uri?: string; readonly issues: { readonly numErrors: number; readonly numWarnings: number; @@ -20,7 +19,6 @@ declare module "gltf-validator" { } export interface ValidationOptions { - readonly uri?: string; readonly maxIssues?: number; readonly ignoredIssues?: readonly string[]; readonly externalResourceFunction?: (uri: string) => Promise; diff --git a/tests/e2e/cli.test.ts b/tests/e2e/cli.test.ts index d090722..3075f35 100644 --- a/tests/e2e/cli.test.ts +++ b/tests/e2e/cli.test.ts @@ -280,7 +280,7 @@ describe("Node Assets CLI", () => { const result = await runNodeAsync([launcher, "pipeline", source, "validate", output], directory); expect(result.code).toBe(0); - expect(result.stdout).toContain(`\u2705 ${source} is valid`); + expect(result.stdout).toContain("\u2705 glTF is valid"); expect(result.stdout).toContain("[Warning]"); expect(result.stdout).toContain("at /nodes/1"); expect((await readGlbAsync(output)).json.meshes).toHaveLength(1); diff --git a/tests/integration/validate.test.ts b/tests/integration/validate.test.ts index c1e8089..c812978 100644 --- a/tests/integration/validate.test.ts +++ b/tests/integration/validate.test.ts @@ -24,24 +24,22 @@ describe("document validation", () => { expect(result).toBe(document); expect(await io.writeJSON(document)).toEqual(before); - expect(readOutput()).toBe("\u2705 scene.glb is valid"); + expect(readOutput()).toBe("\u2705 glTF is valid"); }); - it("prints the URI label and groups informational issues by message and location", async () => { + it("groups informational issues by message and location", async () => { const document = await new NodeIO().readJSON({ json: JSON.parse(generateGltfJson()), resources: {} }); document.createNode(); document.createNode(); const readOutput = captureOutput(); - const result = await new NodeAsset({ name: "grouped-diagnostics", outputBlock: new ValidateBlock({ input: document, uri: "box%20(1).glb" }) }).executeAsync(); + const result = await new NodeAsset({ name: "grouped-diagnostics", outputBlock: new ValidateBlock({ input: document }) }).executeAsync(); expect(result).toBe(document); expect(readOutput()).toBe( - [ - "\u2705 box%20(1).glb is valid", - "[Warning] This object may be unused.\n at /nodes/2\n at /nodes/1", - "[Warning] Empty node encountered.\n at /nodes/2\n at /nodes/1", - ].join("\n\n") + ["\u2705 glTF is valid", "[Warning] This object may be unused.\n at /nodes/2\n at /nodes/1", "[Warning] Empty node encountered.\n at /nodes/2\n at /nodes/1"].join( + "\n\n" + ) ); }); @@ -56,7 +54,7 @@ describe("document validation", () => { const result = await new NodeAsset({ name: "warning", outputBlock: new ValidateBlock({ input: document }) }).executeAsync(); expect(result).toBe(document); - expect(readOutput()).toBe("\u2705 scene.glb is valid\n\n[Warning] Image format not recognized.\n at /images/0"); + expect(readOutput()).toBe("\u2705 glTF is valid\n\n[Warning] Image format not recognized.\n at /images/0"); }); it("logs warnings and informational issues even when validation fails", async () => { @@ -146,7 +144,7 @@ describe("document validation", () => { expect(result).toBe(document); expect(result.hasExtension(extension.extensionName)).toBe(true); - expect(readOutput()).toBe("\u2705 scene.glb is valid"); + expect(readOutput()).toBe("\u2705 glTF is valid"); }); it("validates supported extensions on a directly supplied document", async () => {