Blocks: Replace store name string with exposed store definition #27336

rafaelgallani · 2020-11-27T15:55:19Z

Description

Addresses #27088. Replaces the core/blocks store name references (hardcoded strings) with the exposed store definitions. Had to change some core/blocks unit tests too, since they're testing the exposed definition now.

How has this been tested?

Just making sure npm run test wasn't failing.

Screenshots

Types of changes

Code refactoring, non-breaking change.

Checklist:

My code is tested.
My code follows the WordPress code style.
My code follows the accessibility standards.
My code has proper inline documentation.
I've included developer documentation if appropriate.
I've updated all React Native files affected by any refactorings/renamings in this PR.

rafaelgallani · 2020-11-27T16:32:55Z

That's weird... Apparently, the single test file that failed in the E2E tests runs fine here:

System information

I will try triggering the checks once more with an empty commit...

rafaelgallani · 2020-11-27T17:32:13Z

It failed again. I tried rerunning the tests locally, apparently, it's all good:

As text:

❯ $( npm bin )/wp-scripts test-e2e --config=./packages/e2e-tests/jest.config.js --cacheDirectory="$HOME/.jest-cache" --runTestsByPath $( awk 'NR % 4 == 2' < ~/.jest-e2e-tests )
 PASS  packages/e2e-tests/specs/local/demo.test.js (9.162s)
 PASS  packages/e2e-tests/specs/editor/various/writing-flow.test.js (117.21s)
 PASS  packages/e2e-tests/specs/editor/various/autosave.test.js (66.434s)
 PASS  packages/e2e-tests/specs/experiments/navigation.test.js (38.856s)
 PASS  packages/e2e-tests/specs/editor/plugins/plugins-api.test.js (37.439s)
 PASS  packages/e2e-tests/specs/editor/blocks/heading.test.js (32.808s)
 PASS  packages/e2e-tests/specs/experiments/multi-entity-editing.test.js (30.002s)
 PASS  packages/e2e-tests/specs/editor/various/preview.test.js (27.179s)
 PASS  packages/e2e-tests/specs/editor/various/sidebar.test.js (22.311s)
 PASS  packages/e2e-tests/specs/editor/plugins/container-blocks.test.js (21.492s)
 PASS  packages/e2e-tests/specs/editor/various/taxonomies.test.js (17.924s)
 PASS  packages/e2e-tests/specs/editor/blocks/classic.test.js (18.551s)
 PASS  packages/e2e-tests/specs/editor/plugins/inner-blocks-allowed-blocks.test.js (18.041s)
 PASS  packages/e2e-tests/specs/editor/various/publish-panel.test.js (13.283s)
 PASS  packages/e2e-tests/specs/experiments/multi-entity-saving.test.js (13.033s)
 PASS  packages/e2e-tests/specs/editor/various/nux.test.js (13.401s)
 PASS  packages/e2e-tests/specs/editor/various/navigable-toolbar.test.js (13.913s)
 PASS  packages/e2e-tests/specs/editor/various/duplicating-blocks.test.js (11.204s)
 PASS  packages/e2e-tests/specs/editor/blocks/spacer.test.js (9.35s)
 PASS  packages/e2e-tests/specs/editor/various/compatibility-classic-editor.test.js (9.759s)
 PASS  packages/e2e-tests/specs/editor/various/preferences.test.js (7.388s)
 PASS  packages/e2e-tests/specs/editor/blocks/cover.test.js (7.263s)
 PASS  packages/e2e-tests/specs/editor/blocks/columns.test.js (6.628s)
 PASS  packages/e2e-tests/specs/editor/plugins/custom-taxonomies.test.js (6.123s)
 PASS  packages/e2e-tests/specs/editor/various/adding-patterns.test.js (6.076s)
 PASS  packages/e2e-tests/specs/editor/various/fullscreen-mode.test.js (5.173s)

Test Suites: 26 passed, 26 total
Tests:       113 passed, 113 total
Snapshots:   61 passed, 61 total
Time:        580.458s

I'm not sure why they're failing in the checks...

rafaelgallani · 2020-11-28T21:08:39Z

I just tried:

Rebuilding everything using npm run wp-env destroy, rm -rf build, npm ci and npm run build;
Emulating slow CPU and internet connection as oriented here.

Same results, all tests passed.

I also checked manually one of the errors that happened in the build, and apparently, it's all good; the element exists:

I'm open to ideas. I have no clue what's happening. 😞

gziolo · 2020-11-28T22:46:01Z

Feel free to ignore those failing e2e tests, they shouldn't be relevant to your changes. As far as I remember, there is PR that tries to make them more stable on CI.

rafaelgallani · 2020-11-28T22:48:04Z

Umm... Yeah. Sorry... I should've checked the E2E status before commenting/testing. Apparently, #27347 will fix it 😸

gziolo

Everything looks good, thank you for another great PR 👍

gziolo · 2020-11-30T08:14:13Z

packages/blocks/src/api/raw-handling/test/utils.js


 jest.mock( '@wordpress/data', () => {
 	return {
 		select: jest.fn( ( store ) => {
 			switch ( store ) {
-				case 'core/blocks': {
+				case [ mockStoreName ]:


Quite tricky, but it makes a lot of sense 👍

I could be replacing the switch statement to an if if that's better 😅.

I was commenting about the whole process of mocking. It is it what it is, we better leave it 😃

rafaelgallani added 6 commits November 27, 2020 12:30

Removing internal references to the store name

a6a1a8d

Removing block-library package references to the store name

81a7483

Removing edit-post package references to the store name

38ce873

Removing edit-site package references to the store name

3c41997

Removing editor package references to the store name

7e6f3b1

Removing block-editor package references to the store name

43b7ba2

rafaelgallani requested review from ajitbohra, ellatrix and talldan as code owners November 27, 2020 15:55

Try: Triggering checks/E2E tests

5e64382

gziolo self-requested a review November 27, 2020 18:01

gziolo added [Type] Code Quality Issues or PRs that relate to code quality [Package] Blocks /packages/blocks labels Nov 27, 2020

gziolo approved these changes Nov 30, 2020

View reviewed changes

gziolo mentioned this pull request Nov 30, 2020

Code quality: Replace hardcoded store keys with store definitions #27088

Closed

17 tasks

gziolo approved these changes Nov 30, 2020

View reviewed changes

gziolo merged commit 2239c5d into WordPress:master Nov 30, 2020

github-actions bot added this to the Gutenberg 9.5 milestone Nov 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Blocks: Replace store name string with exposed store definition #27336

Blocks: Replace store name string with exposed store definition #27336

rafaelgallani commented Nov 27, 2020

rafaelgallani commented Nov 27, 2020 •

edited

Loading

rafaelgallani commented Nov 27, 2020

rafaelgallani commented Nov 28, 2020

gziolo commented Nov 28, 2020

rafaelgallani commented Nov 28, 2020

gziolo left a comment

gziolo Nov 30, 2020

rafaelgallani Nov 30, 2020 •

edited

Loading

gziolo Nov 30, 2020

Blocks: Replace store name string with exposed store definition #27336

Blocks: Replace store name string with exposed store definition #27336

Conversation

rafaelgallani commented Nov 27, 2020

Description

How has this been tested?

Screenshots

Types of changes

Checklist:

rafaelgallani commented Nov 27, 2020 • edited Loading

rafaelgallani commented Nov 27, 2020

rafaelgallani commented Nov 28, 2020

gziolo commented Nov 28, 2020

rafaelgallani commented Nov 28, 2020

gziolo left a comment

Choose a reason for hiding this comment

gziolo Nov 30, 2020

Choose a reason for hiding this comment

rafaelgallani Nov 30, 2020 • edited Loading

Choose a reason for hiding this comment

gziolo Nov 30, 2020

Choose a reason for hiding this comment

rafaelgallani commented Nov 27, 2020 •

edited

Loading

rafaelgallani Nov 30, 2020 •

edited

Loading