Migrate native tests to workspace#77425
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
the argument forwarding has been resolved in this. The logs ( generated a bash script to test arg forwarding )--------------------------------------
1. test:native -- --testPathPattern
Expected: jest receives --testPathPattern=<pattern>
--------------------------------------
> gutenberg@23.0.0-rc.1 test:native
> npm run --workspace @wordpress/native-tests test:native -- --testPathPattern=packages/react-native-editor --passWithNoTests
> @wordpress/native-tests@0.0.0 test:native
> cross-env NODE_ENV=test jest --config jest.config.js --testPathPattern=packages/react-native-editor --passWithNoTests
--------------------------------------
2. test:native -- --testNamePattern
Expected: jest receives --testNamePattern=<pattern>
--------------------------------------
> gutenberg@23.0.0-rc.1 test:native
> npm run --workspace @wordpress/native-tests test:native -- --testNamePattern=should render --passWithNoTests
> @wordpress/native-tests@0.0.0 test:native
> cross-env NODE_ENV=test jest --config jest.config.js --testNamePattern=should render --passWithNoTests
--------------------------------------
3. test:native -- --verbose
Expected: jest receives --verbose
--------------------------------------
> gutenberg@23.0.0-rc.1 test:native
> npm run --workspace @wordpress/native-tests test:native -- --verbose --passWithNoTests
> @wordpress/native-tests@0.0.0 test:native
> cross-env NODE_ENV=test jest --config jest.config.js --verbose --passWithNoTests
--------------------------------------
4. test:native -- --runInBand
Expected: jest receives --runInBand
--------------------------------------
> gutenberg@23.0.0-rc.1 test:native
> npm run --workspace @wordpress/native-tests test:native -- --runInBand --passWithNoTests
> @wordpress/native-tests@0.0.0 test:native
> cross-env NODE_ENV=test jest --config jest.config.js --runInBand --passWithNoTests
--------------------------------------
5. test:native:update -- --testPathPattern (critical: hardcoded + forwarded args)
Expected: jest receives --updateSnapshot AND --testPathPattern
--------------------------------------
> gutenberg@23.0.0-rc.1 test:native:update
> npm run --workspace @wordpress/native-tests test:native:update -- --testPathPattern=packages/react-native-editor --passWithNoTests
> @wordpress/native-tests@0.0.0 test:native:update
> npm run test:native -- --updateSnapshot --testPathPattern=packages/react-native-editor --passWithNoTests
> @wordpress/native-tests@0.0.0 test:native
> cross-env NODE_ENV=test jest --config jest.config.js --updateSnapshot --testPathPattern=packages/react-native-editor --passWithNoTests
--------------------------------------
6. test:native:watch (skipped - interactive)
Expected: jest receives --watch
--------------------------------------
--------------------------------------
7. test:native:clean
Expected: jest --clearCache runs
--------------------------------------
> gutenberg@23.0.0-rc.1 test:native:clean
> npm run --workspace @wordpress/native-tests test:native:clean && rm -rf $TMPDIR/jest_*
> @wordpress/native-tests@0.0.0 test:native:clean
> jest --clearCache --config jest.config.jsthe script#!/usr/bin/env bash
# Test native arg forwarding cases.
# Only shows the expanded command npm resolves — suppresses actual test output.
BOLD="\033[1m"
RESET="\033[0m"
run_case() {
local num="$1"
local desc="$2"
local expected="$3"
shift 3
echo ""
echo "--------------------------------------"
echo -e "${BOLD}${num}. ${desc}${RESET}"
echo " Expected: ${expected}"
echo "--------------------------------------"
# Run the command, only show lines npm prints before handing off to jest
# (the "> script" and "> command" lines), suppress everything after
"$@" 2>&1 | grep -E "^>" || true
echo ""
}
echo ""
echo "======================================"
echo " Native Test Arg Forwarding Test Cases"
echo "======================================"
run_case 1 \
"test:native -- --testPathPattern" \
"jest receives --testPathPattern=<pattern>" \
npm run test:native -- --testPathPattern='packages/react-native-editor' --passWithNoTests
run_case 2 \
"test:native -- --testNamePattern" \
"jest receives --testNamePattern=<pattern>" \
npm run test:native -- --testNamePattern='should render' --passWithNoTests
run_case 3 \
"test:native -- --verbose" \
"jest receives --verbose" \
npm run test:native -- --verbose --passWithNoTests
run_case 4 \
"test:native -- --runInBand" \
"jest receives --runInBand" \
npm run test:native -- --runInBand --passWithNoTests
run_case 5 \
"test:native:update -- --testPathPattern (critical: hardcoded + forwarded args)" \
"jest receives --updateSnapshot AND --testPathPattern" \
npm run test:native:update -- --testPathPattern='packages/react-native-editor' --passWithNoTests
run_case 6 \
"test:native:watch (skipped - interactive)" \
"jest receives --watch" \
echo " Skipped (interactive mode)"
run_case 7 \
"test:native:clean" \
"jest --clearCache runs" \
npm run test:native:clean
echo "======================================"
echo " Done. Review expanded commands above."
echo "======================================"
echo "" |
|
Since the integration migration PR has been merged, we can review this one (in free time) and then close Phase 1. cc: @manzoorwanijk |
There was a problem hiding this comment.
Pull request overview
Migrates the native (React Native) Jest test suite into its own workspace package (@wordpress/native-tests) to reduce reliance on the monorepo root dependencies and align with the broader “tests/tools as workspaces” initiative.
Changes:
- Added
test/native/package.jsonto define the new@wordpress/native-testsworkspace and its test scripts/dependencies. - Updated
test/native/jest.config.jsto resolve globs relative to the repo root and includetsxin the package index glob. - Updated root
package.json/package-lock.jsonto register the new workspace and routetest:native:*scripts through it (and remove root-levelreassure).
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/native/package.json | Introduces the @wordpress/native-tests workspace package with native test scripts and dependencies. |
| test/native/jest.config.js | Adjusts path/glob resolution to behave correctly when run from a workspace context. |
| package.json | Adds test/native to workspaces and updates test:native:* scripts to use --workspace @wordpress/native-tests. |
| package-lock.json | Reflects the new workspace and dependency moves (including removing root reassure). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Linting is currently broken in trunk. We will need another trunk merge after #77888 is shipped. |
manzoorwanijk
left a comment
There was a problem hiding this comment.
Apart from the inline comments/suggestions, let us add a minimal readme for the workspace.
| "react-dom": "18.3.1", | ||
| "react-scanner": "1.2.0", | ||
| "reassure": "0.7.1", | ||
| "resize-observer-polyfill": "1.5.1", |
There was a problem hiding this comment.
Why do we need resize-observer-polyfill in the root? I think we can remove it.
| "test:native": "cross-env NODE_ENV=test jest --config jest.config.js", | ||
| "test:native:watch": "npm run test:native -- --watch", | ||
| "test:native:clean": "jest --clearCache --config jest.config.js", | ||
| "test:native:debug": "cross-env NODE_ENV=test jest --runInBand --verbose --config jest.config.js", |
There was a problem hiding this comment.
test:native:debug no longer attaches a debugger
Trunk:
"test:native:debug": "cross-env NODE_ENV=test node --inspect-brk node_modules/.bin/jest --runInBand --verbose --config test/native/jest.config.js"
This PR
"test:native:debug": "cross-env NODE_ENV=test jest --runInBand --verbose --config jest.config.js"
Both node --inspect-brk and the explicit node_modules/.bin/jest are gone. Without --inspect-brk the V8 inspector is never started and there is nothing to attach a debugger to — the :debug variant is now identical in behavior to a plain verbose run.
| "@testing-library/react-native": "^12.0.0", | ||
| "babel-jest": "^29.7.0", | ||
| "cross-env": "^7.0.3", | ||
| "glob": "^10.3.10", |
There was a problem hiding this comment.
The rest of the repo uses "glob": "7.1.2", let us the same version with a version range
| "glob": "^10.3.10", | |
| "glob": "^7.1.2", |
| "gutenberg", | ||
| "native-tests" | ||
| ], | ||
| "homepage": "https://github.com/WordPress/gutenberg/tree/HEAD/test/native/README.md", |
There was a problem hiding this comment.
This readme does not exist. Let us create it but point the homepage simply to the directory.
| "homepage": "https://github.com/WordPress/gutenberg/tree/HEAD/test/native/README.md", | |
| "homepage": "https://github.com/WordPress/gutenberg/tree/HEAD/test/native", |
| "bugs": { | ||
| "url": "https://github.com/WordPress/gutenberg/issues" | ||
| }, | ||
| "devDependencies": { |
There was a problem hiding this comment.
There still are many dependencies which are imported in the workspace but not imported. Let us ensure to declare all of the dependencies which are imported.
I asked Claude to scan for them and here is the list
Imported but NOT declared on the workspace
| Dep | Where used | Currently resolves via |
|---|---|---|
@wordpress/api-fetch |
helpers | hoist from packages/react-native-editor |
@wordpress/block-editor |
helpers, tests | hoist (workspace package) |
@wordpress/block-library |
helpers, tests | hoist (workspace package) |
@wordpress/blocks |
tests | hoist (workspace package) |
@wordpress/core-data |
helpers | hoist (workspace package) |
@wordpress/data |
helpers, tests | hoist (workspace package) |
@wordpress/edit-post |
helpers | hoist from packages/react-native-editor |
@wordpress/element |
helpers, tests | hoist (workspace package) |
@wordpress/react-native-bridge |
helpers, mocks | hoist from packages/react-native-editor |
react |
helper | hoist from root |
react-native |
setup, mocks | hoist from packages/react-native-editor |
react-native-gesture-handler |
setup.js | hoist from packages/react-native-editor |
react-native-reanimated |
setup.js + helpers | hoist from packages/react-native-editor |
react-native-safe-area-context |
setup.js | hoist from packages/react-native-editor |
uuid |
helpers | hoist (multiple packages declare it) |
jest-matcher-utils |
matchers/ | hoist from packages/jest-console |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "devDependencies": { | ||
| "@emotion/jest": "^11.14.2", | ||
| "@testing-library/react-native": "^12.0.0", | ||
| "@wordpress/api-fetch": "file:../../packages/api-fetch", | ||
| "@wordpress/block-editor": "file:../../packages/block-editor", | ||
| "@wordpress/block-library": "file:../../packages/block-library", | ||
| "@wordpress/blocks": "file:../../packages/blocks", | ||
| "@wordpress/core-data": "file:../../packages/core-data", | ||
| "@wordpress/data": "file:../../packages/data", | ||
| "@wordpress/edit-post": "file:../../packages/edit-post", | ||
| "@wordpress/element": "file:../../packages/element", | ||
| "@wordpress/react-native-bridge": "file:../../packages/react-native-bridge", | ||
| "babel-jest": "^29.7.0", | ||
| "cross-env": "^7.0.3", | ||
| "glob": "^7.1.2", | ||
| "jest": "^29.6.2", | ||
| "jest-junit": "^13.0.0", | ||
| "jest-matcher-utils": "^29.6.2", | ||
| "jest-watch-typeahead": "^2.2.2", | ||
| "react": "18.3.1", | ||
| "react-native": "0.73.3", | ||
| "react-native-gesture-handler": "2.14.1", | ||
| "react-native-reanimated": "3.6.2", | ||
| "react-native-safe-area-context": "4.8.2", | ||
| "reassure": "^0.7.1", | ||
| "uuid": "11.1.1" |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "devDependencies": { | ||
| "@babel/plugin-transform-async-generator-functions": "^7.25.7", | ||
| "@babel/plugin-transform-dynamic-import": "^7.25.7", | ||
| "@babel/plugin-transform-export-namespace-from": "^7.25.7", | ||
| "@babel/plugin-transform-react-jsx": "^7.25.7", | ||
| "@babel/plugin-transform-react-jsx-source": "^7.27.1", | ||
| "@babel/plugin-transform-runtime": "^7.25.7", | ||
| "@emotion/jest": "^11.14.2", | ||
| "@react-native/babel-preset": "0.73.10", | ||
| "@testing-library/react-native": "^12.0.0", | ||
| "@wordpress/api-fetch": "file:../../packages/api-fetch", | ||
| "@wordpress/block-editor": "file:../../packages/block-editor", | ||
| "@wordpress/block-library": "file:../../packages/block-library", | ||
| "@wordpress/blocks": "file:../../packages/blocks", | ||
| "@wordpress/core-data": "file:../../packages/core-data", | ||
| "@wordpress/data": "file:../../packages/data", | ||
| "@wordpress/edit-post": "file:../../packages/edit-post", | ||
| "@wordpress/element": "file:../../packages/element", | ||
| "@wordpress/react-native-bridge": "file:../../packages/react-native-bridge", | ||
| "babel-jest": "^29.7.0", | ||
| "babel-plugin-react-native-platform-specific-extensions": "1.1.1", | ||
| "cross-env": "^7.0.3", | ||
| "glob": "^7.1.2", | ||
| "jest": "^29.6.2", | ||
| "jest-junit": "^13.0.0", | ||
| "jest-matcher-utils": "^29.6.2", | ||
| "jest-watch-typeahead": "^2.2.2", | ||
| "react": "18.3.1", | ||
| "react-native": "0.73.3", | ||
| "react-native-gesture-handler": "2.14.1", | ||
| "react-native-reanimated": "3.6.2", | ||
| "react-native-safe-area-context": "4.8.2", | ||
| "reassure": "^0.7.1", | ||
| "uuid": "11.1.1" |
| "test:native": "cross-env NODE_ENV=test jest --config jest.config.js", | ||
| "test:native:watch": "npm run test:native -- --watch", | ||
| "test:native:clean": "jest --clearCache --config jest.config.js", | ||
| "test:native:debug": "cross-env NODE_ENV=test node --inspect-brk node_modules/.bin/jest --runInBand --verbose --config jest.config.js", |
|
Let us address Copilot's last two comments - #77425 (review), then we are good to go. |
manzoorwanijk
left a comment
There was a problem hiding this comment.
Thank you for the changes. This looks great now.
| "react-native-safe-area-context": "4.8.2", | ||
| "react-native-svg": "14.0.0", | ||
| "reassure": "^0.7.1", | ||
| "uuid": "11.1.1" |
There was a problem hiding this comment.
After #77960, we won't need uuid here. Let us get rid of it.
There was a problem hiding this comment.
got it removing it
What?
Part of #75041
Why?
The issue provides more context, but in short, this PR ensures the workspace package does not rely on root dependencies once dependency isolation is enforced.
How?
This PR converts tests/native into a new workspace
@wordpress/native-testswhile still relating the functionality.Testing Instructions
run
npm run test:native:*