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

test: metro-compat resolution tests #524

Merged
merged 47 commits into from Apr 2, 2024
Merged

test: metro-compat resolution tests #524

merged 47 commits into from Apr 2, 2024

Conversation

jbroma
Copy link
Member

@jbroma jbroma commented Mar 18, 2024

Summary

This PR introduces metro-compat package which houses tests suites designed for checking compatibility with parts of metro.

Right now, tests include resolver tests directly imported from metro-resolver from here

Test environment is modified to accommodate for how enhanced-resolve works. Basic principle is this: use unchanged tests and provide compatible resolve function for tests. While in metro-resolver everything happens via mocked methods for checking whether file exists, our tests create a fresh in-memory filesystem for each test to test resolution against, which yields more ensuring results.

Some tests suite are skipped:

  • assets-test.js - because enhanced-resolve, like node resolution, returns only a single entry, it's impossible to support these test cases. We handle this differently using AssetResolver which provides you with first matching entry for development, but all scales are exposed to the runtime
  • index-test.js - this suite contains some basic tests & failure tests which we could include but most of it contains tests for unsupported features like:
    • resolveRequest - this is essentially a custom resolver or a plugin, it should be documented how to achieve similar functionality in Re.Pack
    • redirectModulePath - this is equal to alias field in enhanced-resolve, if end user wants to do this dynamically, then custom resolver/plugin should be used for that - also needs to documented
    • disableHierarchicalLookup - can be achieved in enhanced-resolve by using modules - also needs to be documented

Some test cases are skipped (all come from package-exports-test.js):

  • [nonstrict] tests are skipped because we don't support fallback in form of legacy file resolution
  • asset tests with package exports enabled are skipped for the same reason as above (regarding the assets)
  • rest of the test skipped are due to inline snapshot not matching up with what enhanced-resolve returns - this could be potentially fixed by trapping toThrowErrorMatchingInlineSnapshot calls and checking for existence of errors only

Tests will be excluded from CI for now until everything is aligned properly and some issues are brought up with metro team. Alignment of getResolveOptions will be done in a separate PR. Documentation will follow in a separate PR as well.

Side-note: this PR also unveiled few issues with package exports implementation in metro-resolver which will be investigated more throughly and perhaps become more aligned in the future.

Side-note 2: the way we skip tests without modyfing the test files is rather hacky, we trap jest.describe, jest.it & jest.test and we check if the test name matches, TBD whether there is a better way to do this.

Checklist

  • metro-resolver test suite
  • setup mocks
  • setup skip list
  • exclude tests from turborepo so they are not run with the rest of the tests
  • cleanup
  • README.md in metro-compat

Copy link

changeset-bot bot commented Mar 18, 2024

⚠️ No Changeset found

Latest commit: d7378d9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@jbroma jbroma changed the title [WIP] resolution: metro-compat tests tests: metro-compat resolution tests Mar 18, 2024
@jbroma jbroma marked this pull request as ready for review March 18, 2024 13:53
@jbroma jbroma changed the title tests: metro-compat resolution tests test: metro-compat resolution tests Mar 18, 2024
@jbroma
Copy link
Member Author

jbroma commented Mar 19, 2024

Issue raised with metro: facebook/metro#1236

@jbroma jbroma requested a review from RafikiTiki March 20, 2024 13:09
@jbroma
Copy link
Member Author

jbroma commented Mar 20, 2024

@RafikiTiki don't mind the enabled tests failing for now, it's expected without reworking implementation

@jbroma jbroma merged commit 1587991 into v4 Apr 2, 2024
3 checks passed
@jbroma jbroma deleted the fix-resolution-setup branch April 2, 2024 13:35
jbroma added a commit that referenced this pull request Apr 5, 2024
* fix: use metro defaults for getResolveOptions

* fix: align conditonNames with @react-native/metro-config

* fix: use platform extensions only when not using package exports

* feat: setup environment for metro tests

* refactor: use imported utils

* refactor: use ts for non-imported files

* refactor: move metro tests to a separate workspace package

* chore: remove extra dev deps from repack, update lockfile

* chore: remove changes from eslintrc in repack

* chore: remove changes in repack package setup

* fix: eslint setup

* feat: add typing for ResolutionContext

* refactor: use utils mock

* chore: add enhanced-resolve for types

* feat: setup resolve function in tests

* chore: add @types/jest to metro-compat

* feat: use imported utils module instead of own implementation

* refactor: adjust resolve-error impl

* feat: pass symlinks tests

* feat: add platform-extensions-test

* feat: add package-exports tests

* fix: don't apply js plugins to ts files

* refactor: use package json type from type-fest

* refactor: obtain fileMap from createPackageAccessors instead

* refactor: use separate fields for fileMaps

* feat: add assets & browser-spec tests

* refactor: skip assets tests

* feat: support more package exports cases

* feat: add a list of tests to skip

* feat: add index-tests

* refactor: cleanup resolve mock

* refactor: use babel-jest directly, skip assets & index tests

* refactor: cleanup jest setup

* refactor: add 2 more tests to skip list

* chore: exclude metro-compat from turbo test task

* chore: revert changes to getResolveOptions in this PR

* chore: cleanup in test script

* chore: fix .prettierignore setup

* chore: cleanup .eslintrc.js in metro-compat

* chore: alter formatting and restore tests from upstream

* chore: cleanup & add some utility scripts to metro-compat

* fix: align to target getResolveOptions API

* chore: rename to metro-compat-test

* chore: add README.md for metro-compat-test

* chore: update pnpm lockfile

* fix: update scripts in root package.json to reflect name change
jbroma added a commit that referenced this pull request Apr 5, 2024
* fix: use metro defaults for getResolveOptions

* fix: align conditonNames with @react-native/metro-config

* fix: use platform extensions only when not using package exports

* feat: setup environment for metro tests

* refactor: use imported utils

* refactor: use ts for non-imported files

* refactor: move metro tests to a separate workspace package

* chore: remove extra dev deps from repack, update lockfile

* chore: remove changes from eslintrc in repack

* chore: remove changes in repack package setup

* fix: eslint setup

* feat: add typing for ResolutionContext

* refactor: use utils mock

* chore: add enhanced-resolve for types

* feat: setup resolve function in tests

* chore: add @types/jest to metro-compat

* feat: use imported utils module instead of own implementation

* refactor: adjust resolve-error impl

* feat: pass symlinks tests

* feat: add platform-extensions-test

* feat: add package-exports tests

* fix: don't apply js plugins to ts files

* refactor: use package json type from type-fest

* refactor: obtain fileMap from createPackageAccessors instead

* refactor: use separate fields for fileMaps

* feat: add assets & browser-spec tests

* refactor: skip assets tests

* feat: support more package exports cases

* feat: add a list of tests to skip

* feat: add index-tests

* refactor: cleanup resolve mock

* refactor: use babel-jest directly, skip assets & index tests

* refactor: cleanup jest setup

* refactor: add 2 more tests to skip list

* chore: exclude metro-compat from turbo test task

* chore: revert changes to getResolveOptions in this PR

* chore: cleanup in test script

* chore: fix .prettierignore setup

* chore: cleanup .eslintrc.js in metro-compat

* chore: alter formatting and restore tests from upstream

* chore: cleanup & add some utility scripts to metro-compat

* fix: align to target getResolveOptions API

* chore: rename to metro-compat-test

* chore: add README.md for metro-compat-test

* chore: update pnpm lockfile

* fix: update scripts in root package.json to reflect name change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants