Skip to content
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

[RNMobile] Refactor react-native-editor initialization test #37955

Merged
merged 9 commits into from
Jan 24, 2022
306 changes: 176 additions & 130 deletions packages/react-native-editor/src/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,20 @@ import { render, waitFor } from 'test/helpers';
/**
* WordPress dependencies
*/
import { getBlockTypes, unregisterBlockType } from '@wordpress/blocks';
import * as wpHooks from '@wordpress/hooks';
import '@wordpress/jest-console';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found this library quite useful for assuring we don't produce unexpected logs. In the future, it would be great to enable it globally 🤩 .

An important note about using it is that it silences potential logs that happen after the test. For example, these test cases are generating "act" warnings (i.e. An update to EditorProvider inside a test was not wrapped in act(...).) due to the fact that some store state updates are happening after the test is finished. In the beginning, I thought of this as an issue, but I think it's safe to omit them as they are happening after the test ends.

Copy link
Member

@dcalhoun dcalhoun Jan 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found this library quite useful for assuring we don't produce unexpected logs. In the future, it would be great to enable it globally 🤩 .

Agreed. I would love to enable this globally, we'll just need to address all the current warnings and errors in our test logs first.

An important note about using it is that it silences potential logs that happen after the test.

Yeah, this has confused me before while debugging failing web tests, which do enable @wordpress/jest-console globally. In order to intentionally log information, one has to explicitly assert the log should occur.

For example, these test cases are generating "act" warnings (i.e. An update to EditorProvider inside a test was not wrapped in act(...).) due to the fact that some store state updates are happening after the test is finished. In the beginning, I thought of this as an issue, but I think it's safe to omit them as they are happening after the test ends.

While these particular updates may not dangerous for this specific context, I think it may be dangerous to set the precedent that it is safe to ignore the warnings. It is very possible for a post-assertion update to nullify the validity of a successful assertion. E.g. if one asserts an element is present, a later state update may unexpectedly and erroneously removes the element, which would mean the test is a false positive. The logs occurring "after the test ends" I believe is merely a result of them occurring after the final assertion, which doesn't inherently make them safe.

Is there a way for us to address the act warnings in these tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I would love to enable this globally, we'll just need to address all the current warnings and errors in our test logs first.

In fact, I'm planning to open a ticket specific to addressing this topic.

While these particular updates may not dangerous for this specific context, I think it may be dangerous to set the precedent that it is safe to ignore the warnings. It is very possible for a post-assertion update to nullify the validity of a successful assertion. E.g. if one asserts an element is present, a later state update may unexpectedly and erroneously remove the element, which would mean the test is a false positive. The logs occurring "after the test ends" I believe is merely a result of them occurring after the final assertion, which doesn't inherently make them safe.

Good point, I understand and share your concern about the potential side effects of ignoring the warnings. However, using the @wordpress/jest-console package would make it very easy to ignore them, so it would be great to discuss whether including (although its benefits) it would be safe in relation to this issue. I guess the main problem here is how to know when the editor and its components finish triggering all the state updates, although I'd like to note that it's also important that we identify potential errors upon unmounting the editor or components, in order to prevent memory leaks caused by unclosed listeners.

Being this said, I'm thinking of creating a native version of the supported matchers and excluding the error logs. This way we would assure the "act" errors (note that they're warnings but logged via console.error) are logged hence, we could easily identify them in the output.

Is there a way for us to address the act warnings in these tests?

Actually, I managed to find a workaround by delaying the query passed to the waitFor function an execution block cycle (check the rnmobile/add/use-immediate-wait-for branch). I haven't investigated the original issue, but I have the gut feeling that it might be related to how the store actions and updates are executed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, using the @wordpress/jest-console package would make it very easy to ignore them, so it would be great to discuss whether including (although its benefits) it would be safe in relation to this issue.

The absence of @wordpress/jest-console makes it far easier for warnings and errors to go unnoticed, as they will not necessarily cause test failures. I agree @wordpress/jest-console makes it easier to explicitly hide warnings and errors. I would posit the former is worst. The latter is at least an intentional act.

I guess the main problem here is how to know when the editor and its components finish triggering all the state updates, although I'd like to note that it's also important that we identify potential errors upon unmounting the editor or components, in order to prevent memory leaks caused by unclosed listeners.

Agreed. We appear to be aligned that it is not safe or good practice to ignore the warnings or assume they are safe.

Being this said, I'm thinking of creating a native version of the supported matchers and excluding the error logs. This way we would assure the "act" errors (note that they're warnings but logged via console.error) are logged hence, we could easily identify them in the output.

Maybe I am mistaken, but I interpret the intent of @wordpress/jest-console is to fail tests when console logs, warnings, or errors occur. act errors would trigger test failures when @wordpress/jest-console is enabled. So, @wordpress/jest-console itself should already accomplish your proposed end goal: ensure act errors are not overlooked. I.e. someone is far more likely to notice — and hopefully fix — a failing test (due to an act error) than a passing test that happens to also log an act error.

Example Failure from act + @wordpress/jest-console
 FAIL  ../block-library/src/cover/test/edit.native.js (7.967 s)
  when no media is attached
    ○ skipped adds an image or video
  when an image is attached
    ✕ discards canceled focal point changes (343 ms)
    ○ skipped edits the image
    ○ skipped replaces the image
    ○ skipped clears the image within image edit button
    ○ skipped toggles a fixed background
    ○ skipped edits the focal point with a slider
    ○ skipped edits the focal point with a text input
    ○ skipped clears the media within cell button
  color settings
    ○ skipped sets a color for the overlay background when the placeholder is visible
    ○ skipped sets a gradient overlay background when a solid background was already selected
    ○ skipped toggles between solid colors and gradients
    ○ skipped clears the selected overlay color and mantains the inner blocks

  ● when an image is attached › discards canceled focal point changes

    expect(jest.fn()).not.toHaveErrored(expected)

    Expected mock function not to be called but it was called with:
    ["Warning: An update to %s inside a test was not wrapped in act(...).·
    When testing, code that causes React state updates should be wrapped into act(...):·
    act(() => {
      /* fire events that update state */
    });
    /* assert on the output */·
    This ensures that you're testing the behavior the user would see in the browser. Learn more at https://reactjs.org/link/wrap-tests-with-act%s", "ForwardRef(NavigationContainer)", "
        at NavigationContainer (/Users/[REDACTED]/Sites/a8c/gutenberg/node_modules/@react-navigation/native/lib/commonjs/NavigationContainer.tsx:40:5)
        at View
        at View (/Users/[REDACTED]/Sites/a8c/gutenberg/node_modules/react-native/jest/mockComponent.js:28:18)
        at BottomSheetNavigationContainer (/Users/[REDACTED]/Sites/a8c/gutenberg/packages/components/src/mobile/bottom-sheet/bottom-sheet-navigation/navigation-container.native.js:63:2)
        at View
        at View (/Users/[REDACTED]/Sites/a8c/gutenberg/node_modules/react-native/jest/mockComponent.js:28:18)
        at View
        at View (/Users/[REDACTED]/Sites/a8c/gutenberg/node_modules/react-native/jest/mockComponent.js:28:18)
        at KeyboardAvoidingView (/Users/[REDACTED]/Sites/a8c/gutenberg/packages/components/src/mobile/bottom-sheet/keyboard-avoiding-view.native.js:26:16)
        at Modal
        at /Users/[REDACTED]/Sites/a8c/gutenberg/test/native/setup.js:88:8
        at BottomSheet (/Users/[REDACTED]/Sites/a8c/gutenberg/packages/components/src/mobile/bottom-sheet/index.native.js:51:16)
        at BottomSheetSettings (/Users/[REDACTED]/Sites/a8c/gutenberg/packages/block-editor/src/components/block-settings/container.native.js:29:2)
        at WithDispatch(BottomSheetSettings) (/Users/[REDACTED]/Sites/a8c/gutenberg/packages/data/src/components/with-dispatch/index.js:95:26)
        at /Users/[REDACTED]/Sites/a8c/gutenberg/packages/data/src/components/with-select/index.js:56:24
        at WithSelect(WithDispatch(BottomSheetSettings)) (/Users/[REDACTED]/Sites/a8c/gutenberg/packages/compose/src/higher-order/pure/index.tsx:35:3)
        at SlotFillProvider (/Users/[REDACTED]/Sites/a8c/gutenberg/packages/components/src/slot-fill/provider.js:18:16)
        at CoverEdit"],["Warning: An update to %s inside a test was not wrapped in act(...).·
    When testing, code that causes React state updates should be wrapped into act(...):·
    act(() => {
      /* fire events that update state */
    });
    /* assert on the output */·
    This ensures that you're testing the behavior the user would see in the browser. Learn more at https://reactjs.org/link/wrap-tests-with-act%s", "Cover", "
        at Cover (/Users/[REDACTED]/Sites/a8c/gutenberg/packages/block-library/src/cover/edit.native.js:83:2)
        at WithDispatch(Component) (/Users/[REDACTED]/Sites/a8c/gutenberg/packages/data/src/components/with-dispatch/index.js:95:26)
        at /Users/[REDACTED]/Sites/a8c/gutenberg/packages/data/src/components/with-select/index.js:56:24
        at WithSelect(WithDispatch(Component)) (/Users/[REDACTED]/Sites/a8c/gutenberg/packages/compose/src/higher-order/pure/index.tsx:35:3)
        at Edit (/Users/[REDACTED]/Sites/a8c/gutenberg/packages/block-editor/src/components/block-edit/edit.native.js:29:10)
        at WithToolbarControls(Edit) (/Users/[REDACTED]/Sites/a8c/gutenberg/packages/block-editor/src/hooks/align.js:119:11)
        at WithInspectorControl(WithToolbarControls(Edit)) (/Users/[REDACTED]/Sites/a8c/gutenberg/packages/block-editor/src/hooks/anchor.js:68:45)
        at WithToolbarControls(WithInspectorControl(WithToolbarControls(Edit))) (/Users/[REDACTED]/Sites/a8c/gutenberg/packages/block-editor/src/hooks/style.js:262:33)
        at WithFilters(Edit) (/Users/[REDACTED]/Sites/a8c/gutenberg/packages/components/src/higher-order/with-filters/index.js:49:18)
        at BlockEdit (/Users/[REDACTED]/Sites/a8c/gutenberg/packages/block-editor/src/components/block-edit/index.js:23:10)
        at SlotFillProvider (/Users/[REDACTED]/Sites/a8c/gutenberg/packages/components/src/slot-fill/provider.js:18:16)
        at CoverEdit"]

      34 |      function assertExpectedCalls() {
      35 |              if ( spy.assertionsNumber === 0 && spy.mock.calls.length > 0 ) {
    > 36 |                      expect( console ).not[ matcherName ]();
         |                      ^
      37 |              }
      38 |      }
      39 |

      at Object.assertExpectedCalls (packages/jest-console/src/index.js:36:4)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 12 skipped, 13 total
Snapshots:   0 total
Time:        9.12 s
Ran all test suites matching /packages\/block-library\/src\/cover\/test\/edit.native.js/i.

Actually, I managed to find a workaround by delaying the query passed to the waitFor function an execution block cycle (check the rnmobile/add/use-immediate-wait-for branch). I haven't investigated the original issue, but I have the gut feeling that it might be related to how the store actions and updates are executed.

Interesting. I have limited knowledge of the error, so I am uncertain to the appropriateness of the possible fix. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I am mistaken, but I interpret the intent of @wordpress/jest-console is to fail tests when console logs, warnings, or errors occur. act errors would trigger test failures when @wordpress/jest-console is enabled. So, @wordpress/jest-console itself should already accomplish your proposed end goal: ensure act errors are not overlooked. I.e. someone is far more likely to notice — and hopefully fix — a failing test (due to an act error) than a passing test that happens to also log an act error.

That's correct, however, it doesn't identify the act warnings that happen after the test finishes, which are the ones that we wouldn't notice if we enable @wordpress/jest-console.

Let me share the following table, in order to make clearer the benefits and drawbacks of whether enable @wordpress/jest-console:

Using @wordpress/jest-console Not using @wordpress/jest-console
Benefits:
- act warnings will make the test fail hence, it will require contributors to fix them.
- Other log types will also be required to be addressed, which would help in having more robust tests.

Drawbacks:
- Any type of log that happens before or after a test won't be displayed and either considered a test failure. This implies that for component rendering, some act warnings will be unnoticed.
Benefits:
- We won't experience the drawbacks of using @wordpress/jest-console related to omitting warnings or error logs out of the scope of tests.

Drawbacks:
- Nevertheless, the logs won't produce test failures, which would require manual observation of test output.
- For some tests, the output might be too verbose, unless we mock console functions for those cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dcalhoun I removed the use of @wordpress/jest-console package in 4e08331, note that it required a small refactor to cover the same logic. I'd appreciate it if you could take a look and let me know your thoughts about that approach, thanks 🙇 .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like using Jest v27 would help on this issue, per this comment, I noticed that a recently added test is failing due to an act warning being logged after the test is finished 🤩 , and includes the @wordpress/jest-console (this is the first test we have introduced it).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct, however, it doesn't identify the act warnings that happen after the test finishes, which are the ones that we wouldn't notice if we enable @wordpress/jest-console.

Interesting. I was not aware of that. That is really surprising. I wonder why that is. My understanding of act warnings was that they specifically warn about changes that occur after the test finishes. @wordpress/jest-console "swallows" all act warnings — both during and after tests — but only fails the test for act warnings that occur "during" the test?

@dcalhoun I removed the use of @wordpress/jest-console package in 4e08331, note that it required a small refactor to cover the same logic. I'd appreciate it if you could take a look and let me know your thoughts about that approach, thanks 🙇 .

I think the new approach is an appropriate solution while we further investigate more global, long-term solutions.

Looks like using Jest v27 would help on this issue, per this comment, I noticed that a recently added test is failing due to an act warning being logged after the test is finished 🤩 , and includes the @wordpress/jest-console (this is the first test we have introduced it).

👀 Following along that conversation and exploration as well. 👍🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I was not aware of that. That is really surprising. I wonder why that is. My understanding of act warnings was that they specifically warn about changes that occur after the test finishes. @wordpress/jest-console "swallows" all act warnings — both during and after tests — but only fails the test for act warnings that occur "during" the test?

Exactly, only fails for act warnings that occur during the test. However, when using Jest v27, I noticed that this behavior changed and also makes the test fail when those warnings happen after the test 🤷‍♂️ .

I think the new approach is an appropriate solution while we further investigate more global, long-term solutions.

Great, I'll proceed with it, and we'll see how we could improve it in the next iteration 👍.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heads up that I re-enabled this import as with Jest v27 the act warnings that occur after the test are also identified as errors.


/**
* Internal dependencies
*/
import { registerGutenberg } from '..';
import { registerGutenberg, initialHtmlGutenberg } from '..';
import setupLocale from '../setup-locale';

jest.mock( 'react-native/Libraries/ReactNative/AppRegistry' );
jest.mock( '../setup-locale' );

const initGutenberg = ( registerParams ) => {
const initGutenberg = ( registerParams, editorProps ) => {
let EditorComponent;
AppRegistry.registerComponent.mockImplementation(
( name, componentProvider ) => {
Expand All @@ -27,158 +29,202 @@ const initGutenberg = ( registerParams ) => {
);
registerGutenberg( registerParams );

return render( <EditorComponent /> );
let screen;
// This guarantees that setup module is imported on every test, as it's imported upon Editor component rendering.
jest.isolateModules( () => {
screen = render( <EditorComponent { ...editorProps } /> );
} );
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this approach, we no longer need to reset modules for testing the call order of hooks and callbacks. Besides, I found out that resetting modules was breaking the test cases that render the editor.


return screen;
};

describe( 'Register Gutenberg', () => {
beforeEach( () => {
// We need to reset modules to guarantee that setup module is imported on every test.
jest.resetModules();
} );
Comment on lines -34 to -37
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer necessary as the setup module import is guaranteed by using the jest.isolateModules function when rendering the editor.


it( 'registers Gutenberg editor component', () => {
registerGutenberg();
expect( AppRegistry.registerComponent ).toHaveBeenCalled();
} );

it( 'sets up locale before editor is initialized', () => {
const mockOnModuleImported = jest.fn();
jest.mock( '../setup', () => {
// To determine if the setup module is imported, we create a mock function that is called when the module is mocked.
mockOnModuleImported();
describe( 'check calling order of hooks and callbacks', () => {
afterAll( () => {
// Revert setup mock to assure that rest of tests use original implementation.
jest.unmock( '../setup' );
} );

return {
__esModule: true,
default: jest.fn().mockReturnValue( <></> ),
};
it( 'sets up locale before editor is initialized', () => {
const mockOnModuleImported = jest.fn();
// jest.resetModules();
fluiddot marked this conversation as resolved.
Show resolved Hide resolved
jest.mock( '../setup', () => {
// To determine if the setup module is imported, we create a mock function that is called when the module is mocked.
mockOnModuleImported();

return {
__esModule: true,
default: jest.fn().mockReturnValue( <></> ),
};
} );

initGutenberg();

// "invocationCallOrder" can be used to compare call orders between different mocks.
// Reference: https://git.io/JyBk0
const setupLocaleCallOrder =
setupLocale.mock.invocationCallOrder[ 0 ];
const onSetupImportedCallOrder =
mockOnModuleImported.mock.invocationCallOrder[ 0 ];

expect( setupLocaleCallOrder ).toBeLessThan(
onSetupImportedCallOrder
);
} );

initGutenberg();
it( 'beforeInit callback is invoked before the editor is initialized', () => {
const beforeInitCallback = jest.fn();
const mockOnModuleImported = jest.fn();
jest.mock( '../setup', () => {
// To determine if the setup module is imported, we create a mock function that is called when the module is mocked.
mockOnModuleImported();

return {
__esModule: true,
default: jest.fn().mockReturnValue( <></> ),
};
} );

initGutenberg( { beforeInitCallback } );

// "invocationCallOrder" can be used to compare call orders between different mocks.
// Reference: https://git.io/JyBk0
const beforeInitCallOrder =
beforeInitCallback.mock.invocationCallOrder[ 0 ];
const onSetupImportedCallOrder =
mockOnModuleImported.mock.invocationCallOrder[ 0 ];

expect( beforeInitCallOrder ).toBeLessThan(
onSetupImportedCallOrder
);
} );

// "invocationCallOrder" can be used to compare call orders between different mocks.
// Reference: https://git.io/JyBk0
const setupLocaleCallOrder = setupLocale.mock.invocationCallOrder[ 0 ];
const onSetupImportedCallOrder =
mockOnModuleImported.mock.invocationCallOrder[ 0 ];
it( 'dispatches "native.pre-render" hook before the editor is rendered', () => {
const doAction = jest.spyOn( wpHooks, 'doAction' );

expect( setupLocaleCallOrder ).toBeLessThan( onSetupImportedCallOrder );
} );
// An empty component is provided in order to listen for render calls of the editor component.
const onRenderEditor = jest.fn();
const EditorComponent = () => {
onRenderEditor();
return null;
};
jest.mock( '../setup', () => ( {
__esModule: true,
default: jest.fn().mockReturnValue( <EditorComponent /> ),
} ) );

initGutenberg();

const hookCallIndex = 0;
// "invocationCallOrder" can be used to compare call orders between different mocks.
// Reference: https://git.io/JyBk0
const hookCallOrder =
doAction.mock.invocationCallOrder[ hookCallIndex ];
const onRenderEditorCallOrder =
onRenderEditor.mock.invocationCallOrder[ 0 ];
const hookName = doAction.mock.calls[ hookCallIndex ][ 0 ];

expect( hookName ).toBe( 'native.pre-render' );
expect( hookCallOrder ).toBeLessThan( onRenderEditorCallOrder );
} );

it( 'beforeInit callback is invoked before the editor is initialized', () => {
const beforeInitCallback = jest.fn();
const mockOnModuleImported = jest.fn();
jest.mock( '../setup', () => {
// To determine if the setup module is imported, we create a mock function that is called when the module is mocked.
mockOnModuleImported();
it( 'dispatches "native.block_editor_props" hook before the editor is rendered', () => {
const applyFilters = jest.spyOn( wpHooks, 'applyFilters' );

return {
// An empty component is provided in order to listen for render calls of the editor component.
const onRenderEditor = jest.fn();
const EditorComponent = () => {
onRenderEditor();
return null;
};
jest.mock( '../setup', () => ( {
__esModule: true,
default: jest.fn().mockReturnValue( <></> ),
default: jest.fn().mockReturnValue( <EditorComponent /> ),
} ) );

initGutenberg();

const hookCallIndex = 0;
// "invocationCallOrder" can be used to compare call orders between different mocks.
// Reference: https://git.io/JyBk0
const hookCallOrder =
applyFilters.mock.invocationCallOrder[ hookCallIndex ];
const onRenderEditorCallOrder =
onRenderEditor.mock.invocationCallOrder[ 0 ];
const hookName = applyFilters.mock.calls[ hookCallIndex ][ 0 ];

expect( hookName ).toBe( 'native.block_editor_props' );
expect( hookCallOrder ).toBeLessThan( onRenderEditorCallOrder );
} );

it( 'dispatches "native.render" hook after the editor is rendered', () => {
const doAction = jest.spyOn( wpHooks, 'doAction' );

// An empty component is provided in order to listen for render calls of the editor component.
const onRenderEditor = jest.fn();
const EditorComponent = () => {
onRenderEditor();
return null;
};
jest.mock( '../setup', () => ( {
__esModule: true,
default: jest.fn().mockReturnValue( <EditorComponent /> ),
} ) );

initGutenberg();

const hookCallIndex = 1;
// "invocationCallOrder" can be used to compare call orders between different mocks.
// Reference: https://git.io/JyBk0
const hookCallOrder =
doAction.mock.invocationCallOrder[ hookCallIndex ];
const onRenderEditorCallOrder =
onRenderEditor.mock.invocationCallOrder[ 0 ];
const hookName = doAction.mock.calls[ hookCallIndex ][ 0 ];

expect( hookName ).toBe( 'native.render' );
expect( hookCallOrder ).toBeGreaterThan( onRenderEditorCallOrder );
} );
} );

initGutenberg( { beforeInitCallback } );
describe( 'editor initialization', () => {
beforeEach( () => {
// Setup already registers blocks so we need assure that no blocks are registered before the test.
getBlockTypes().forEach( ( block ) => {
unregisterBlockType( block.name );
} );
} );

// "invocationCallOrder" can be used to compare call orders between different mocks.
// Reference: https://git.io/JyBk0
const beforeInitCallOrder =
beforeInitCallback.mock.invocationCallOrder[ 0 ];
const onSetupImportedCallOrder =
mockOnModuleImported.mock.invocationCallOrder[ 0 ];
it( 'initializes the editor with empty HTML', async () => {
const { getByTestId } = initGutenberg( {}, { initialData: '' } );

expect( beforeInitCallOrder ).toBeLessThan( onSetupImportedCallOrder );
} );
const blockList = await waitFor( () =>
getByTestId( 'block-list-wrapper' )
);
fluiddot marked this conversation as resolved.
Show resolved Hide resolved

it( 'dispatches "native.pre-render" hook before the editor is rendered', () => {
const doAction = jest.spyOn( wpHooks, 'doAction' );

// An empty component is provided in order to listen for render calls of the editor component.
const onRenderEditor = jest.fn();
const EditorComponent = () => {
onRenderEditor();
return null;
};
jest.mock( '../setup', () => ( {
__esModule: true,
default: jest.fn().mockReturnValue( <EditorComponent /> ),
} ) );

initGutenberg();

const hookCallIndex = 0;
// "invocationCallOrder" can be used to compare call orders between different mocks.
// Reference: https://git.io/JyBk0
const hookCallOrder =
doAction.mock.invocationCallOrder[ hookCallIndex ];
const onRenderEditorCallOrder =
onRenderEditor.mock.invocationCallOrder[ 0 ];
const hookName = doAction.mock.calls[ hookCallIndex ][ 0 ];

expect( hookName ).toBe( 'native.pre-render' );
expect( hookCallOrder ).toBeLessThan( onRenderEditorCallOrder );
} );
expect( blockList ).toHaveProperty( 'type', 'View' );
expect( console ).toHaveLogged( 'Hermes is: true' );
} );

it( 'dispatches "native.block_editor_props" hook before the editor is rendered', () => {
const applyFilters = jest.spyOn( wpHooks, 'applyFilters' );

// An empty component is provided in order to listen for render calls of the editor component.
const onRenderEditor = jest.fn();
const EditorComponent = () => {
onRenderEditor();
return null;
};
jest.mock( '../setup', () => ( {
__esModule: true,
default: jest.fn().mockReturnValue( <EditorComponent /> ),
} ) );

initGutenberg();

const hookCallIndex = 0;
// "invocationCallOrder" can be used to compare call orders between different mocks.
// Reference: https://git.io/JyBk0
const hookCallOrder =
applyFilters.mock.invocationCallOrder[ hookCallIndex ];
const onRenderEditorCallOrder =
onRenderEditor.mock.invocationCallOrder[ 0 ];
const hookName = applyFilters.mock.calls[ hookCallIndex ][ 0 ];

expect( hookName ).toBe( 'native.block_editor_props' );
expect( hookCallOrder ).toBeLessThan( onRenderEditorCallOrder );
} );
it( 'initializes the editor with initial HTML', async () => {
const { getByTestId } = initGutenberg(
{},
{ initialData: initialHtmlGutenberg }
);

it( 'dispatches "native.render" hook after the editor is rendered', () => {
const doAction = jest.spyOn( wpHooks, 'doAction' );

// An empty component is provided in order to listen for render calls of the editor component.
const onRenderEditor = jest.fn();
const EditorComponent = () => {
onRenderEditor();
return null;
};
jest.mock( '../setup', () => ( {
__esModule: true,
default: jest.fn().mockReturnValue( <EditorComponent /> ),
} ) );

initGutenberg();

const hookCallIndex = 1;
// "invocationCallOrder" can be used to compare call orders between different mocks.
// Reference: https://git.io/JyBk0
const hookCallOrder =
doAction.mock.invocationCallOrder[ hookCallIndex ];
const onRenderEditorCallOrder =
onRenderEditor.mock.invocationCallOrder[ 0 ];
const hookName = doAction.mock.calls[ hookCallIndex ][ 0 ];

expect( hookName ).toBe( 'native.render' );
expect( hookCallOrder ).toBeGreaterThan( onRenderEditorCallOrder );
} );
const blockList = await waitFor( () =>
getByTestId( 'block-list-wrapper' )
);
fluiddot marked this conversation as resolved.
Show resolved Hide resolved

it( 'initializes the editor', () => {
const { getByTestId } = initGutenberg();
const blockList = waitFor( () => getByTestId( 'block-list-wrapper' ) );
expect( blockList ).toBeDefined();
expect( blockList ).toHaveProperty( 'type', 'View' );
fluiddot marked this conversation as resolved.
Show resolved Hide resolved
expect( console ).toHaveLogged( 'Hermes is: true' );
// It's expected that some blocks are upgraded and inform about it (example: "Updated Block: core/cover")
expect( console ).toHaveInformed();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be giving us a clue about using outdated versions of blocks in initialHtml file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By not passing arguments to toHaveInformed, I would imagine we expose ourselves to additional console.info calls accumulating without our knowledge. From reviewing the source of @wordpress/jest-console, I suppose there is no way to further constrain this assertion without passing the entirety of the explicit block update output, which is not manageable unfortunately.

This might be giving us a clue about using outdated versions of blocks in initialHtml file.

With this are you implying that we should fix the initialHtml rather than suppressing the console.info message here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By not passing arguments to toHaveInformed, I would imagine we expose ourselves to additional console.info calls accumulating without our knowledge. From reviewing the source of @wordpress/jest-console, I suppose there is no way to further constrain this assertion without passing the entirety of the explicit block update output, which is not manageable unfortunately.

Exactly, there might be some cases where you could overlook extra or unexpected info logs using this function without passing arguments. In the case of this test, although we know the expected log output, I decided not to include it due to its size.

With this are you implying that we should fix the initialHtml rather than suppressing the console.info message here?

Yep, I haven't investigated this further, but the fact that some blocks are being upgraded makes me wonder whether they should be reviewed, in case the HTML we're using for them is old.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case, I think we could address this in a different PR, I'll open a ticket for this as a follow-up.

} );
} );
} );