diff --git a/blocks/api/test/parser.js b/blocks/api/test/parser.js index 3eb16a2878e59..5ed001bb49940 100644 --- a/blocks/api/test/parser.js +++ b/blocks/api/test/parser.js @@ -1,8 +1,3 @@ -/** - * External dependencies - */ -import { noop } from 'lodash'; - /** * Internal dependencies */ @@ -32,6 +27,18 @@ describe( 'block parser', () => { title: 'block title', }; + const unknownBlockSettings = { + category: 'common', + title: 'unknown block', + attributes: { + content: { + type: 'string', + source: 'html', + }, + }, + save: ( { attributes } ) => attributes.content, + }; + afterEach( () => { setUnknownTypeHandlerName( undefined ); getBlockTypes().forEach( ( block ) => { @@ -163,7 +170,7 @@ describe( 'block parser', () => { const block = createBlockWithFallback( 'core/test-block', - 'content', + 'Bananas', { fruit: 'Bananas' } ); expect( block.name ).toEqual( 'core/test-block' ); @@ -173,49 +180,35 @@ describe( 'block parser', () => { it( 'should create the requested block with no attributes if it exists', () => { registerBlockType( 'core/test-block', defaultBlockSettings ); - const block = createBlockWithFallback( 'core/test-block', 'content' ); + const block = createBlockWithFallback( 'core/test-block', '' ); expect( block.name ).toEqual( 'core/test-block' ); expect( block.attributes ).toEqual( {} ); } ); it( 'should fall back to the unknown type handler for unknown blocks if present', () => { - registerBlockType( 'core/unknown-block', { - category: 'common', - title: 'unknown block', - attributes: { - content: { - type: 'string', - source: 'html', - }, - fruit: { - type: 'string', - }, - }, - save: ( { attributes } ) => attributes.content, - } ); + registerBlockType( 'core/unknown-block', unknownBlockSettings ); setUnknownTypeHandlerName( 'core/unknown-block' ); const block = createBlockWithFallback( 'core/test-block', - 'content', + 'Bananas', { fruit: 'Bananas' } ); expect( block.name ).toBe( 'core/unknown-block' ); - expect( block.attributes.fruit ).toBe( 'Bananas' ); expect( block.attributes.content ).toContain( 'wp:test-block' ); } ); it( 'should fall back to the unknown type handler if block type not specified', () => { - registerBlockType( 'core/unknown-block', defaultBlockSettings ); + registerBlockType( 'core/unknown-block', unknownBlockSettings ); setUnknownTypeHandlerName( 'core/unknown-block' ); const block = createBlockWithFallback( null, 'content' ); expect( block.name ).toEqual( 'core/unknown-block' ); - expect( block.attributes ).toEqual( {} ); + expect( block.attributes ).toEqual( { content: 'content' } ); } ); it( 'should not create a block if no unknown type handler', () => { - const block = createBlockWithFallback( 'core/test-block', 'content' ); + const block = createBlockWithFallback( 'core/test-block', '' ); expect( block ).toBeUndefined(); } ); } ); @@ -232,7 +225,7 @@ describe( 'block parser', () => { url: { type: 'string' }, chicken: { type: 'string' }, }, - save: noop, + save: ( { attributes } ) => attributes.content, category: 'common', title: 'test block', } ); @@ -268,7 +261,7 @@ describe( 'block parser', () => { source: 'text', }, }, - save: noop, + save: ( { attributes } ) => attributes.content, category: 'common', title: 'test block', } ); @@ -291,7 +284,7 @@ describe( 'block parser', () => { registerBlockType( 'core/test-block', defaultBlockSettings ); const parsed = parse( - '\nRibs\n' + '\nBananas\n' ); expect( parsed ).toHaveLength( 1 ); @@ -304,7 +297,7 @@ describe( 'block parser', () => { setUnknownTypeHandlerName( 'core/unknown-block' ); const parsed = parse( - 'Ribs' + + '\nBananas\n' + '

Broccoli

' + 'Ribs' ); @@ -314,12 +307,12 @@ describe( 'block parser', () => { it( 'should parse the post content, using unknown block handler', () => { registerBlockType( 'core/test-block', defaultBlockSettings ); - registerBlockType( 'core/unknown-block', defaultBlockSettings ); + registerBlockType( 'core/unknown-block', unknownBlockSettings ); setUnknownTypeHandlerName( 'core/unknown-block' ); const parsed = parse( - 'Ribs' + + '\nBananas\n' + '

Broccoli

' + 'Ribs' ); @@ -334,25 +327,15 @@ describe( 'block parser', () => { it( 'should parse the post content, including raw HTML at each end', () => { registerBlockType( 'core/test-block', defaultBlockSettings ); - registerBlockType( 'core/unknown-block', { - attributes: { - content: { - type: 'string', - source: 'html', - }, - }, - save: noop, - category: 'common', - title: 'unknown block', - } ); + registerBlockType( 'core/unknown-block', unknownBlockSettings ); setUnknownTypeHandlerName( 'core/unknown-block' ); const parsed = parse( '

Cauliflower

' + - 'Ribs' + + '\nBananas\n' + '\n

Broccoli

\n' + - 'Ribs' + + '\nBananas\n' + '

Romanesco

' ); diff --git a/blocks/api/test/validation.js b/blocks/api/test/validation.js index f203913491685..6e3aabba82169 100644 --- a/blocks/api/test/validation.js +++ b/blocks/api/test/validation.js @@ -30,6 +30,18 @@ describe( 'validation', () => { title: 'block title', }; + /* eslint-disable no-console */ + function expectError() { + expect( console.error ).toHaveBeenCalled(); + console.error.mockClear(); + } + + function expectWarning() { + expect( console.warn ).toHaveBeenCalled(); + console.warn.mockClear(); + } + /* eslint-enable no-console */ + afterEach( () => { setUnknownTypeHandlerName( undefined ); getBlockTypes().forEach( ( block ) => { @@ -102,6 +114,7 @@ describe( 'validation', () => { { chars: 'a \n c \t b ' }, ); + expectWarning(); expect( isEqual ).toBe( false ); } ); @@ -178,7 +191,7 @@ describe( 'validation', () => { expect( isEqual ).toBe( true ); } ); - it( 'returns true if not same style', () => { + it( 'returns false if not same style', () => { const isEqual = isEqualAttributesOfName.style( 'background-image: url( "https://wordpress.org/img.png" ); color: red;', 'color: red; font-size: 13px; background-image: url(\'https://wordpress.org/img.png\');' @@ -201,6 +214,7 @@ describe( 'validation', () => { ] ); + expectWarning(); expect( isEqual ).toBe( false ); } ); @@ -228,6 +242,7 @@ describe( 'validation', () => { { tagName: 'section' } ); + expectWarning(); expect( isEqual ).toBe( false ); } ); @@ -248,6 +263,7 @@ describe( 'validation', () => { } ); + expectWarning(); expect( isEqual ).toBe( false ); } ); @@ -306,6 +322,7 @@ describe( 'validation', () => { '
Hello World!
' ); + expectWarning(); expect( isEquivalent ).toBe( false ); } ); @@ -324,6 +341,7 @@ describe( 'validation', () => { '
Hello' ); + expectWarning(); expect( isEquivalent ).toBe( false ); } ); @@ -333,6 +351,7 @@ describe( 'validation', () => { '
Hello
' ); + expectWarning(); expect( isEquivalent ).toBe( false ); } ); @@ -369,6 +388,7 @@ describe( 'validation', () => { '' ); + expectWarning(); expect( isEquivalent ).toBe( false ); } ); @@ -378,6 +398,7 @@ describe( 'validation', () => { '
' ); + expectWarning(); expect( isEquivalent ).toBe( false ); } ); @@ -387,22 +408,27 @@ describe( 'validation', () => { '
' ); + expectWarning(); expect( isEquivalent ).toBe( false ); } ); } ); describe( 'isValidBlock()', () => { - it( 'returns false is block is not valid', () => { + it( 'returns false if block is not valid', () => { registerBlockType( 'core/test-block', defaultBlockSettings ); - expect( isValidBlock( + const isValid = isValidBlock( 'Apples', getBlockType( 'core/test-block' ), { fruit: 'Bananas' } - ) ).toBe( false ); + ); + + expectWarning(); + expectError(); + expect( isValid ).toBe( false ); } ); - it( 'returns false is error occurs while generating block save', () => { + it( 'returns false if error occurs while generating block save', () => { registerBlockType( 'core/test-block', { ...defaultBlockSettings, save() { @@ -410,21 +436,26 @@ describe( 'validation', () => { }, } ); - expect( isValidBlock( + const isValid = isValidBlock( 'Bananas', getBlockType( 'core/test-block' ), { fruit: 'Bananas' } - ) ).toBe( false ); + ); + + expectError(); + expect( isValid ).toBe( false ); } ); it( 'returns true is block is valid', () => { registerBlockType( 'core/test-block', defaultBlockSettings ); - expect( isValidBlock( + const isValid = isValidBlock( 'Bananas', getBlockType( 'core/test-block' ), { fruit: 'Bananas' } - ) ).toBe( true ); + ); + + expect( isValid ).toBe( true ); } ); } ); } ); diff --git a/blocks/api/validation.js b/blocks/api/validation.js index cf6c99d8374d8..1ea863c9e341d 100644 --- a/blocks/api/validation.js +++ b/blocks/api/validation.js @@ -125,6 +125,36 @@ const MEANINGFUL_ATTRIBUTES = [ ...ENUMERATED_ATTRIBUTES, ]; +/** + * Object of logger functions. + */ +const log = ( () => { + /** + * Creates a logger with block validation prefix. + * + * @param {Function} logger Original logger function + * @return {Function} Augmented logger function + */ + function createLogger( logger ) { + // In test environments, pre-process the sprintf message to improve + // readability of error messages. We'd prefer to avoid pulling in this + // dependency in runtime environments, and it can be dropped by a combo + // of Webpack env substitution + UglifyJS dead code elimination. + if ( process.env.NODE_ENV === 'test' ) { + return ( ...args ) => logger( require( 'sprintf-js' ).sprintf( ...args ) ); + } + + return ( message, ...args ) => logger( 'Block validation: ' + message, ...args ); + } + + return { + /* eslint-disable no-console */ + error: createLogger( console.error ), + warning: createLogger( console.warn ), + /* eslint-enable no-console */ + }; +} )(); + /** * Given a specified string, returns an array of strings split by consecutive * whitespace, ignoring leading or trailing whitespace. @@ -172,16 +202,22 @@ export function getMeaningfulAttributePairs( token ) { * Returns true if two text tokens (with `chars` property) are equivalent, or * false otherwise. * - * @param {Object} a First token - * @param {Object} b Second token - * @return {Boolean} Whether two text tokens are equivalent + * @param {Object} actual Actual token + * @param {Object} expected Expected token + * @return {Boolean} Whether two text tokens are equivalent */ -export function isEqualTextTokensWithCollapsedWhitespace( a, b ) { +export function isEqualTextTokensWithCollapsedWhitespace( actual, expected ) { // This is an overly simplified whitespace comparison. The specification is // more prescriptive of whitespace behavior in inline and block contexts. // // See: https://medium.com/@patrickbrosset/when-does-white-space-matter-in-html-b90e8a7cdd33 - return isEqual( ...[ a.chars, b.chars ].map( getTextWithCollapsedWhitespace ) ); + const isEquivalentText = isEqual( ...[ actual.chars, expected.chars ].map( getTextWithCollapsedWhitespace ) ); + + if ( ! isEquivalentText ) { + log.warning( 'Expected text `%s`, saw `%s`.', expected.chars, actual.chars ); + } + + return isEquivalentText; } /** @@ -230,13 +266,13 @@ export function getStyleProperties( text ) { * @type {Object} */ export const isEqualAttributesOfName = { - class: ( a, b ) => { + class: ( actual, expected ) => { // Class matches if members are the same, even if out of order or // superfluous whitespace between. - return ! xor( ...[ a, b ].map( getTextPiecesSplitOnWhitespace ) ).length; + return ! xor( ...[ actual, expected ].map( getTextPiecesSplitOnWhitespace ) ).length; }, - style: ( a, b ) => { - return isEqual( ...[ a, b ].map( getStyleProperties ) ); + style: ( actual, expected ) => { + return isEqual( ...[ actual, expected ].map( getStyleProperties ) ); }, }; @@ -244,38 +280,42 @@ export const isEqualAttributesOfName = { * Given two sets of attribute tuples, returns true if the attribute sets are * equivalent * - * @param {Array[]} a First attributes tuples - * @param {Array[]} b Second attributes tuples - * @return {Boolean} Whether attributes are equivalent + * @param {Array[]} actual Actual attributes tuples + * @param {Array[]} expected Expected attributes tuples + * @return {Boolean} Whether attributes are equivalent */ -export function isEqualTagAttributePairs( a, b ) { +export function isEqualTagAttributePairs( actual, expected ) { // Attributes is tokenized as tuples. Their lengths should match. This also // avoids us needing to check both attributes sets, since if A has any keys // which do not exist in B, we know the sets to be different. - if ( a.length !== b.length ) { + if ( actual.length !== expected.length ) { + log.warning( 'Expected attributes %o, instead saw %o.', expected, actual ); return false; } // Convert tuples to object for ease of lookup - const [ aAttributes, bAttributes ] = [ a, b ].map( fromPairs ); + const [ actualAttributes, expectedAttributes ] = [ actual, expected ].map( fromPairs ); - for ( const name in aAttributes ) { + for ( const name in actualAttributes ) { // As noted above, if missing member in B, assume different - if ( ! bAttributes.hasOwnProperty( name ) ) { + if ( ! expectedAttributes.hasOwnProperty( name ) ) { + log.warning( 'Encountered unexpected attribute `%s`.', name ); return false; } - const aValue = aAttributes[ name ]; - const bValue = bAttributes[ name ]; + const actualValue = actualAttributes[ name ]; + const expectedValue = expectedAttributes[ name ]; const isEqualAttributes = isEqualAttributesOfName[ name ]; if ( isEqualAttributes ) { // Defer custom attribute equality handling - if ( ! isEqualAttributes( aValue, bValue ) ) { + if ( ! isEqualAttributes( actualValue, expectedValue ) ) { + log.warning( 'Expected attribute `%s` of value `%s`, saw `%s`.', name, expectedValue, actualValue ); return false; } - } else if ( aValue !== bValue ) { + } else if ( actualValue !== expectedValue ) { // Otherwise strict inequality should bail + log.warning( 'Expected attribute `%s` of value `%s`, saw `%s`.', name, expectedValue, actualValue ); return false; } } @@ -289,13 +329,14 @@ export function isEqualTagAttributePairs( a, b ) { * @type {Object} */ export const isEqualTokensOfType = { - StartTag: ( a, b ) => { - if ( a.tagName !== b.tagName ) { + StartTag: ( actual, expected ) => { + if ( actual.tagName !== expected.tagName ) { + log.warning( 'Expected tag name `%s`, instead saw `%s`.', actual.tagName, expected.tagName ); return false; } return isEqualTagAttributePairs( - ...[ a, b ].map( getMeaningfulAttributePairs ) + ...[ actual, expected ].map( getMeaningfulAttributePairs ) ); }, Chars: isEqualTextTokensWithCollapsedWhitespace, @@ -328,13 +369,13 @@ export function getNextNonWhitespaceToken( tokens ) { * Returns true if there is given HTML strings are effectively equivalent, or * false otherwise. * - * @param {String} a First HTML string - * @param {String} b Second HTML string + * @param {String} actual Actual HTML string + * @param {String} expected Expected HTML string * @return {Boolean} Whether HTML strings are equivalent */ -export function isEquivalentHTML( a, b ) { +export function isEquivalentHTML( actual, expected ) { // Tokenize input content and reserialized save content - const [ actualTokens, expectedTokens ] = [ a, b ].map( tokenize ); + const [ actualTokens, expectedTokens ] = [ actual, expected ].map( tokenize ); let actualToken, expectedToken; while ( ( actualToken = getNextNonWhitespaceToken( actualTokens ) ) ) { @@ -342,11 +383,13 @@ export function isEquivalentHTML( a, b ) { // Inequal if exhausted all expected tokens if ( ! expectedToken ) { + log.warning( 'Expected end of content, instead saw %o.', actualToken ); return false; } // Inequal if next non-whitespace token of each set are not same type if ( actualToken.type !== expectedToken.type ) { + log.warning( 'Expected token of type `%s` (%o), instead saw `%s` (%o).', expectedToken.type, expectedToken, actualToken.type, actualToken ); return false; } @@ -361,6 +404,7 @@ export function isEquivalentHTML( a, b ) { while ( ( expectedToken = getNextNonWhitespaceToken( expectedTokens ) ) ) { // If any non-whitespace tokens remain in expected token set, this // indicates inequality + log.warning( 'Expected %o, instead saw end of content.', expectedToken ); return false; } @@ -384,8 +428,20 @@ export function isValidBlock( innerHTML, blockType, attributes ) { try { saveContent = getSaveContent( blockType, attributes ); } catch ( error ) { + log.error( 'Block validation failed because an error occurred while generating block content:\n\n%s', error.toString() ); return false; } - return isEquivalentHTML( innerHTML, saveContent ); + const isValid = isEquivalentHTML( innerHTML, saveContent ); + if ( ! isValid ) { + log.error( + 'Block validation failed for `%s` (%o).\n\nExpected:\n\n%s\n\nActual:\n\n%s', + blockType.name, + blockType, + saveContent, + innerHTML + ); + } + + return isValid; } diff --git a/blocks/test/fixtures/core__gallery.html b/blocks/test/fixtures/core__gallery.html index 37aff1683b508..b74b304f41e3f 100644 --- a/blocks/test/fixtures/core__gallery.html +++ b/blocks/test/fixtures/core__gallery.html @@ -1,5 +1,5 @@ -