-
Notifications
You must be signed in to change notification settings - Fork 385
fix: improve validation error details in validateCommonProjectManifest #2966
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Lil-Duckling-22
wants to merge
10
commits into
subquery:main
Choose a base branch
from
Lil-Duckling-22:fix/improve-validation-error-details
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
b26630d
fix: improve validation error details in validateCommonProjectManifest
Lil-Duckling-22 ff31007
Create load.spec.ts
Lil-Duckling-22 3dc59e7
Update load.ts
Lil-Duckling-22 0616ebe
Update utils.ts
Lil-Duckling-22 8f8b20f
Update base.ts
Lil-Duckling-22 7aa34f3
Update load.spec.ts
Lil-Duckling-22 dfc70a0
Update utils.ts
Lil-Duckling-22 de9ef18
Update load.ts
Lil-Duckling-22 02c3d2a
Update base.ts
Lil-Duckling-22 c5c19c7
Update load.spec.ts
Lil-Duckling-22 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,290 @@ | ||
| // Copyright 2020-2025 SubQuery Pte Ltd authors & contributors | ||
| // SPDX-License-Identifier: GPL-3.0 | ||
|
|
||
| import {validateCommonProjectManifest} from './load'; | ||
|
|
||
| describe('validateCommonProjectManifest', () => { | ||
| /** | ||
| * Example of error output format improvement: | ||
| * | ||
| * BEFORE (old format using toString()): | ||
| * project validation failed. | ||
| * An instance of CommonProjectManifestV1_0_0Impl has failed the validation: | ||
| * - property version has failed the following constraints: isString | ||
| * - property schema has failed the following constraints: nested property schema must be either object or array | ||
| * | ||
| * AFTER (new structured format): | ||
| * project validation failed. | ||
| * - version: version must be a string | ||
| * - schema: schema must be an object | ||
| * - network: network must be an object | ||
| * - runner: runner must be an object | ||
| * - dataSources: dataSources must be an array | ||
| */ | ||
| it('should throw error with structured format for missing required fields', () => { | ||
| const invalidManifest = { | ||
| specVersion: '1.0.0', | ||
| // Missing required fields: version, schema, network, runner, dataSources | ||
| }; | ||
|
|
||
| expect(() => validateCommonProjectManifest(invalidManifest)).toThrow(); | ||
|
|
||
| try { | ||
| validateCommonProjectManifest(invalidManifest); | ||
| fail('Expected validation to throw an error'); | ||
| } catch (error: any) { | ||
| const errorMessage = error.message; | ||
|
|
||
| // Check that error message starts with the expected prefix | ||
| expect(errorMessage).toContain('project validation failed.'); | ||
|
|
||
| // Check that error message contains structured format with property names | ||
| // The new format should include property names and constraints | ||
| expect(errorMessage).toMatch(/ - \w+:/); | ||
|
|
||
| // Verify that multiple properties are listed | ||
| const lines = errorMessage.split('\n').filter((line: string) => line.trim().startsWith('-')); | ||
| expect(lines.length).toBeGreaterThan(0); | ||
|
|
||
| // Each line should follow the format: " - propertyName: constraint message" | ||
| lines.forEach((line: string) => { | ||
| expect(line).toMatch(/^\s+-\s+\w+:\s+.+$/); | ||
| }); | ||
|
|
||
| // Verify specific properties are mentioned | ||
| expect(errorMessage).toMatch(/version|schema|network|runner|dataSources/); | ||
| } | ||
| }); | ||
|
|
||
| it('should throw error with property-specific constraints for invalid specVersion', () => { | ||
| const invalidManifest = { | ||
| specVersion: '2.0.0', // Invalid: must be '1.0.0' | ||
| version: '1.0.0', | ||
| schema: {file: 'schema.graphql'}, | ||
| network: {chainId: '0x123'}, | ||
| runner: { | ||
| node: {name: '@subql/node', version: '1.0.0'}, | ||
| query: {name: '@subql/query', version: '1.0.0'}, | ||
| }, | ||
| dataSources: [], | ||
| }; | ||
|
|
||
| expect(() => validateCommonProjectManifest(invalidManifest)).toThrow(); | ||
|
|
||
| try { | ||
| validateCommonProjectManifest(invalidManifest); | ||
| fail('Expected validation to throw an error'); | ||
| } catch (error: any) { | ||
| const errorMessage = error.message; | ||
|
|
||
| // Should contain specVersion property in error | ||
| expect(errorMessage).toContain('specVersion'); | ||
|
|
||
| // Should contain constraint information in structured format | ||
| expect(errorMessage).toMatch(/specVersion:\s*.+/); | ||
|
|
||
| // Verify the format: " - specVersion: constraint message" | ||
| const specVersionLine = errorMessage.split('\n').find((line: string) => line.includes('specVersion')); | ||
| expect(specVersionLine).toMatch(/^\s+-\s+specVersion:\s+.+$/); | ||
| } | ||
| }); | ||
|
|
||
| it('should throw error with structured format for invalid runner query name', () => { | ||
| const invalidManifest = { | ||
| specVersion: '1.0.0', | ||
| version: '1.0.0', | ||
| schema: {file: 'schema.graphql'}, | ||
| network: {chainId: '0x123'}, | ||
| runner: { | ||
| node: {name: '@subql/node', version: '1.0.0'}, | ||
| query: {name: '@subql/invalid-query', version: '1.0.0'}, // Invalid query name | ||
| }, | ||
| dataSources: [], | ||
| }; | ||
|
|
||
| expect(() => validateCommonProjectManifest(invalidManifest)).toThrow(); | ||
|
|
||
| try { | ||
| validateCommonProjectManifest(invalidManifest); | ||
| fail('Expected validation to throw an error'); | ||
| } catch (error: any) { | ||
| const errorMessage = error.message; | ||
|
|
||
| // Should contain query property in error (nested in runner.query.name) | ||
| expect(errorMessage).toContain('query'); | ||
|
|
||
| // Error should be structured with property path | ||
| expect(errorMessage).toMatch(/query/); | ||
|
|
||
| // Verify structured format for nested property | ||
| const queryLine = errorMessage.split('\n').find((line: string) => line.includes('query')); | ||
| if (queryLine) { | ||
| expect(queryLine).toMatch(/^\s+-\s+query/); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| it('should throw error with structured format for missing chainId', () => { | ||
| const invalidManifest = { | ||
| specVersion: '1.0.0', | ||
| version: '1.0.0', | ||
| schema: {file: 'schema.graphql'}, | ||
| network: { | ||
| // Missing required chainId | ||
| endpoint: 'wss://example.com', | ||
| }, | ||
| runner: { | ||
| node: {name: '@subql/node', version: '1.0.0'}, | ||
| query: {name: '@subql/query', version: '1.0.0'}, | ||
| }, | ||
| dataSources: [], | ||
| }; | ||
|
|
||
| expect(() => validateCommonProjectManifest(invalidManifest)).toThrow(); | ||
|
|
||
| try { | ||
| validateCommonProjectManifest(invalidManifest); | ||
| fail('Expected validation to throw an error'); | ||
| } catch (error: any) { | ||
| const errorMessage = error.message; | ||
|
|
||
| // Should contain chainId or network property in error | ||
| expect(errorMessage).toMatch(/chainId|network/); | ||
|
|
||
| // Should be in structured format | ||
| expect(errorMessage).toMatch(/ - \w+:/); | ||
|
|
||
| // Verify the error format shows property name and constraint | ||
| const chainIdLine = errorMessage.split('\n').find((line: string) => | ||
| line.includes('chainId') || line.includes('network') | ||
| ); | ||
| if (chainIdLine) { | ||
| expect(chainIdLine).toMatch(/^\s+-\s+\w+:\s+.+$/); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| it('should demonstrate improved error message format with actual output', () => { | ||
| const invalidManifest = { | ||
| specVersion: '1.0.0', | ||
| version: '', // Empty string should fail validation | ||
| schema: {file: 'schema.graphql'}, | ||
| network: {chainId: '0x123'}, | ||
| runner: { | ||
| node: {name: '@subql/node', version: '1.0.0'}, | ||
| query: {name: '@subql/query', version: '1.0.0'}, | ||
| }, | ||
| dataSources: [], | ||
| }; | ||
|
|
||
| try { | ||
| validateCommonProjectManifest(invalidManifest); | ||
| fail('Expected validation to throw an error'); | ||
| } catch (error: any) { | ||
| const errorMessage = error.message; | ||
|
|
||
| // The new format should be clear and structured | ||
| // Example output: | ||
| // project validation failed. | ||
| // - version: version must be a string | ||
|
|
||
| expect(errorMessage).toContain('project validation failed.'); | ||
| expect(errorMessage).toContain('version'); | ||
|
|
||
| // Verify the structured format is present | ||
| const versionErrorLine = errorMessage.split('\n').find((line: string) => | ||
| line.includes('version') && line.includes(':') | ||
| ); | ||
| expect(versionErrorLine).toBeDefined(); | ||
| expect(versionErrorLine).toMatch(/^\s+-\s+version:\s+.+$/); | ||
| } | ||
| }); | ||
|
|
||
| it('should not throw error for valid manifest', () => { | ||
| const validManifest = { | ||
| specVersion: '1.0.0', | ||
| version: '1.0.0', | ||
| schema: {file: 'schema.graphql'}, | ||
| network: { | ||
| chainId: '0x123', | ||
| endpoint: 'wss://example.com', | ||
| }, | ||
| runner: { | ||
| node: {name: '@subql/node', version: '1.0.0'}, | ||
| query: {name: '@subql/query', version: '1.0.0'}, | ||
| }, | ||
| dataSources: [], | ||
| }; | ||
|
|
||
| expect(() => validateCommonProjectManifest(validManifest)).not.toThrow(); | ||
| }); | ||
|
|
||
| it('should format errors for nested child objects with array indices', () => { | ||
| // This test demonstrates how errors are formatted for deeply nested objects, | ||
| // such as an invalid filter on a mapping handler. | ||
| // The error path should use bracket notation for array indices: dataSources[0].mapping.handlers[1].filter | ||
| const invalidManifest = { | ||
| specVersion: '1.0.0', | ||
| version: '1.0.0', | ||
| schema: {file: 'schema.graphql'}, | ||
| network: { | ||
| chainId: '0x123', | ||
| endpoint: 'wss://example.com', | ||
| }, | ||
| runner: { | ||
| node: {name: '@subql/node', version: '1.0.0'}, | ||
| query: {name: '@subql/query', version: '1.0.0'}, | ||
| }, | ||
| dataSources: [ | ||
| { | ||
| kind: 'substrate/Runtime', | ||
| mapping: { | ||
| file: 'dist/index.js', | ||
| handlers: [ | ||
| { | ||
| handler: 'handleBlock', | ||
| kind: 'substrate/BlockHandler', | ||
| }, | ||
| { | ||
| handler: 'handleEvent', | ||
| kind: 'substrate/EventHandler', | ||
| filter: { | ||
| // Invalid: specVersion should be an array of 2 numbers, not a single number | ||
| specVersion: 123, | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| ], | ||
| }; | ||
|
|
||
| expect(() => validateCommonProjectManifest(invalidManifest)).toThrow(); | ||
|
|
||
| try { | ||
| validateCommonProjectManifest(invalidManifest); | ||
| fail('Expected validation to throw an error'); | ||
| } catch (error: any) { | ||
| const errorMessage = error.message; | ||
|
|
||
| // Error should contain the structured format | ||
| expect(errorMessage).toContain('project validation failed.'); | ||
|
|
||
| // For nested array errors, the path should use bracket notation | ||
| // Example: dataSources[0].mapping.handlers[1].filter.specVersion | ||
| // Verify that array indices are formatted with brackets | ||
| expect(errorMessage).toMatch(/\[\d+\]/); | ||
|
|
||
| // The error message should be structured and readable with full example | ||
| // Expected format: " - dataSources[0].mapping.handlers[1].filter.specVersion: <constraint message>" | ||
| const errorLines = errorMessage.split('\n').filter((line: string) => line.trim().startsWith('-')); | ||
| expect(errorLines.length).toBeGreaterThan(0); | ||
|
|
||
| // Verify at least one line contains array bracket notation and follows the expected format | ||
| const arrayErrorLine = errorLines.find((line: string) => /\[\d+\]/.test(line)); | ||
| expect(arrayErrorLine).toBeDefined(); | ||
| expect(arrayErrorLine).toMatch(/^\s+-\s+.+\[\d+\].+:\s+.+$/); | ||
| } | ||
| }); | ||
| }); | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All tests that have a try/catch should also have a
failcall. If the function being tested doesn't throw then there will be no expectations.