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.

Sign up for GitHub

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

Fix store crash with gutenberg master #1366

Merged
merged 28 commits into from
Sep 27, 2019
Merged

Fix store crash with gutenberg master #1366

Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
e2defe5
Fix store crash with gutenberg master
Tug Sep 13, 2019
4433387
Update gutenberg ref
Tug Sep 13, 2019
a652ccc
Update gutenberg ref
Tug Sep 19, 2019
8ab38d5
Remove unused require
Tug Sep 20, 2019
fb33f0c
Fix tests
Tug Sep 20, 2019
d0cc756
Merge remote-tracking branch 'origin/develop' into update/fix-store-c…
Tug Sep 20, 2019
8c2c477
Add missing @wordpress/block-directory package in symlinked-packages
Tug Sep 20, 2019
08caa7c
Update gutenberg ref
Tug Sep 20, 2019
1e0ad56
Use real timers for e2e tests
Tug Sep 24, 2019
97da943
Use fake timers in unit tests not config
Tug Sep 24, 2019
03cc901
Update gutenberg ref
Tug Sep 26, 2019
47ed605
Update gutenberg ref
Tug Sep 26, 2019
042ee2b
Merge remote-tracking branch 'origin/develop' into update/fix-store-c…
Tug Sep 26, 2019
79009b1
Add comment explaining why we need to call jest.runAllTicks();
Tug Sep 26, 2019
69c339d
Merge remote-tracking branch 'origin/develop' into update/fix-store-c…
Tug Sep 26, 2019
b651fbe
Update gutenberg ref
Tug Sep 26, 2019
e7e7b7b
Merge remote-tracking branch 'origin/develop' into update/fix-store-c…
Tug Sep 26, 2019
20a565c
Increase the number of workers so tests don't get queued when calling…
Tug Sep 27, 2019
3b5577f
Reenable disabled e2e tests
Tug Sep 27, 2019
6c75768
Reenable disabled e2e tests
Tug Sep 27, 2019
8c9f4cb
Update gutenberg ref
Tug Sep 27, 2019
81168f5
Update gutenberg ref
Tug Sep 27, 2019
fb6db57
Disable warning notices
Tug Sep 27, 2019
561f27e
Disable warning notices, fix lint
Tug Sep 27, 2019
078e62d
Merge remote-tracking branch 'origin/develop' into update/fix-store-c…
Tug Sep 27, 2019
0081ee8
Update gutenberg ref
Tug Sep 27, 2019
be87adf
Revert max workers to 3 and increase timeout in tests for now
Tug Sep 27, 2019
b55f55d
Update gutenberg ref
Tug Sep 27, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion __device-tests__/gutenberg-editor-lists.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import testData from './helpers/test-data';

jasmine.DEFAULT_TIMEOUT_INTERVAL = 400000;

xdescribe( 'Gutenberg Editor tests for List block', () => {
describe( 'Gutenberg Editor tests for List block', () => {
let driver;
let editorPage;
let allPassed = true;
Expand Down
2 changes: 1 addition & 1 deletion __device-tests__/gutenberg-editor-rotatation.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import testData from './helpers/test-data';

jasmine.DEFAULT_TIMEOUT_INTERVAL = 300000;

xdescribe( 'Gutenberg Editor tests', () => {
describe( 'Gutenberg Editor tests', () => {
let driver;
let editorPage;
let allPassed = true;
Expand Down
2 changes: 1 addition & 1 deletion gutenberg
Open in desktop
Submodule gutenberg updated 776 files
1 change: 1 addition & 0 deletions jest_ui.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const main = require( './jest.config.js' );

module.exports = {
...main,
timers: 'real',
setupFiles: [],
testMatch: [ '**/__device-tests__/**/*.test.[jt]s?(x)' ],
testPathIgnorePatterns: [
Expand Down
6 changes: 3 additions & 3 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@
"test": "cross-env NODE_ENV=test jest --verbose --config jest.config.js",
"test:inside-gb": "cross-env NODE_ENV=test jest --verbose --config jest_gb.config.js",
"test:debug": "cross-env NODE_ENV=test node --inspect-brk jest --runInBand --verbose --config jest.config.js",
"device-tests": "cross-env NODE_ENV=test jest --no-cache --maxWorkers=3 --reporters=default --reporters=jest-junit --verbose --config jest_ui.config.js",
"device-tests": "cross-env NODE_ENV=test jest --no-cache --maxWorkers=8 --reporters=default --reporters=jest-junit --verbose --config jest_ui.config.js",
"device-tests:local": "cross-env NODE_ENV=test jest --runInBand --reporters=default --reporters=jest-junit --detectOpenHandles --verbose --config jest_ui.config.js",
"device-tests:debug": "cross-env NODE_ENV=test node $NODE_DEBUG_OPTION --inspect-brk node_modules/jest/bin/jest --runInBand --reporters=default --reporters=jest-junit --detectOpenHandles --verbose --config jest_ui.config.js",
"test:e2e": "yarn test:e2e:android && yarn test:e2e:ios",
Expand Down Expand Up @@ -147,8 +147,8 @@
"node-libs-react-native": "^1.0.2",
"node-sass": "^4.12.0",
"react": "16.8.6",
"react-native": "jtreanor/react-native#v0.60.0-patched",
"react-native-dark-mode": "git+https://github.com/wordpress-mobile/react-native-dark-mode.git#f09bf1480e7b34536413ab3300f29e4375edb2c6",
"react-native": "jtreanor/react-native#v0.60.0-patched",
"react-native-dark-mode": "git+https://github.com/wordpress-mobile/react-native-dark-mode.git#f09bf1480e7b34536413ab3300f29e4375edb2c6",
"react-native-hr": "git+https://github.com/Riglerr/react-native-hr.git#2d01a5cf77212d100e8b99e0310cce5234f977b3",
"react-native-keyboard-aware-scroll-view": "git+https://github.com/wordpress-mobile/react-native-keyboard-aware-scroll-view.git#gb-v0.8.7",
"react-native-modal": "^6.5.0",
Expand Down
11 changes: 4 additions & 7 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
/**
* External dependencies
*/
import { AppRegistry, I18nManager, YellowBox } from 'react-native';
import { AppRegistry, I18nManager } from 'react-native';
import React from 'react';

/**
Expand All @@ -23,12 +23,8 @@ import { getTranslation } from '../i18n-cache';
import initialHtml from './initial-html';

const gutenbergSetup = () => {
const apiFetch = require( '@wordpress/api-fetch' ).default;
const wpData = require( '@wordpress/data' );

// wp-api-fetch
apiFetch.use( apiFetch.createRootURLMiddleware( 'https://public-api.wordpress.com/' ) );

// wp-data
const userId = 1;
const storageKey = 'WP_DATA_USER_' + userId;
Expand Down Expand Up @@ -82,8 +78,9 @@ export class RootComponent extends React.Component {
}

export function registerApp() {
// Disable require circle warnings showing up in the app (they will still be visible in the console)
YellowBox.ignoreWarnings( [ 'Require cycle:' ] );
// Disable warnings as they disrupt the user experience in dev mode
// eslint-disable-next-line no-console
console.disableYellowBox = true;

gutenbergSetup();

Expand Down
13 changes: 10 additions & 3 deletions src/index.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,25 @@ describe( 'RootComponent', () => {
beforeAll( initializeEditor );

it( 'renders without crashing', () => {
jest.useFakeTimers();
const app = renderer.create( <RootComponent /> );
// Gutenberg store currently has some asynchronous parts in the store setup
// Need to run all ticks so `isReady` is true in the editor store
// See: https://github.com/wordpress-mobile/gutenberg-mobile/pull/1366#discussion_r326813061
renderer.act( () => {
jest.runAllTicks();
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting!
Can we add a comment saying why/when is this needed? 🤔

Copy link
Contributor Author

@Tug Tug Sep 20, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Added one here for the same reason.

It seems that when EDIT_ENTITY_RECORD is dispatched using this generator function it's done so asynchronously, meaning the promise does not resolve immediately even during tests.

Which makes resetEditorBlocks async which means setupEditorState which comes right after is delayed which means isReady will be true after some time which means editorDidMount will also be called after some time.

This is related to the changes made on core data entities so I figured it was safe to update our tests for now. I'll try to find a good explanation for this and add something 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

This is very inconvenient to enforce running fake timers but I guess this is what it is. I don't think we use unit tests to test the root component for the web code.

} );
const rendered = app.toJSON();
expect( rendered ).toBeTruthy();
app.unmount();
} );

it( 'renders without crashing with a block focused', () => {
const app = renderer.create( <RootComponent /> );
const blocks = select( 'core/block-editor' ).getBlocks();

// Methods that modify state are required to be called inside `act`
renderer.act( () => {
jest.runAllTicks();
const blocks = select( 'core/block-editor' ).getBlocks();
// Methods that modify state are required to be called inside `act`
dispatch( 'core/block-editor' ).selectBlock( blocks[ 0 ].clientId );
} );

Expand Down
1 change: 1 addition & 0 deletions symlinked-packages/@wordpress/block-directory
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
../../gutenberg/packages/block-directory/src