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

fix(jest): Fix watchmode on api tests #6438

Merged
merged 4 commits into from
Sep 24, 2022

Conversation

dac09
Copy link
Contributor

@dac09 dac09 commented Sep 22, 2022

...by loading scenarios in the same context as jest.

This unfortunately is a compromise, because the work I did to improve the memory usage of our scenario tests gets a little nullified now.

What is the change?

One of the memory optimisations I did was to make sure scenarios are loaded and seeded outside the context/VM that the tests run in. This seemed to really help with memory usage, but unfortunately breaks tests in watch mode, with a cryptic DB/Prisma error (on Postgres):

    ConnectorError(ConnectorError { user_facing_error: None, kind: QueryError(Error { kind: Db, cause: Some(DbError { severity: "ERROR", parsed_severity: Some(Error), code: SqlState(EXX000), message: "cache lookup failed for type 226432", detail: None, hint: None, position: None, where_: Some("unnamed portal parameter $3"), schema: None, table: None, column: None, datatype: None, constraint: None, file: Some("lsyscache.c"), line: Some(2904), routine: Some("getTypeBinaryInputInfo") }) }) })

This PR now puts back the scenario loading and seeding back in the same context, while keeping some of the other minor optimisations, as well as the tests that don't use scenarios being fast.

What do I do if my api side tests run out of memory on CI?

One of the things we can suggest is to use Jest 28+'s sharding.

e.g.

yarn rw test api --no-watch --shard 1/3
yarn rw test api --no-watch --shard 2/3
yarn rw test api --no-watch --shard 3/3

(adjust the number of shards as needed. 50 or so test-suites per shard is a good number!)

This will allow tests to run to completion

@dac09 dac09 added the release:fix This PR is a fix label Sep 22, 2022
@dthyresson
Copy link
Contributor

@dac09 Do you think we should add a tip in https://redwoodjs.com/docs/testing#jest with:

What do I do if my api side tests run out of memory on CI?

One of the things we can suggest is to use Jest 28+'s sharding.

e.g.

yarn rw test api --no-watch --shard 1/3
yarn rw test api --no-watch --shard 2/3
yarn rw test api --no-watch --shard 3/3

(adjust the number of shards as needed. 50 or so test-suites per shard is a good number!)

This will allow tests to run to completion.


Or words to that effect?

Copy link
Contributor Author

dac09 commented Sep 22, 2022

Sure - I just don’t know if that’s the right place to put it. Because it’s most likely something you see in CI.

@jtoar jtoar added this to the next-release-patch milestone Sep 22, 2022
@jtoar jtoar enabled auto-merge (squash) September 24, 2022 00:23
@jtoar jtoar disabled auto-merge September 24, 2022 01:04
@jtoar jtoar merged commit f78f8eb into redwoodjs:main Sep 24, 2022
@jtoar jtoar modified the milestones: next-release-patch, v3.0.2 Sep 24, 2022
jtoar pushed a commit that referenced this pull request Sep 24, 2022
jtoar pushed a commit that referenced this pull request Sep 24, 2022
jtoar pushed a commit that referenced this pull request Sep 24, 2022
virtuoushub added a commit to redwoodjs/example-storybook that referenced this pull request Nov 6, 2022
@virtuoushub
Copy link
Collaborator

fwiw - I am seeing odd test errors that may or may not be related to this PR:

...
FAIL api api/src/services/comments/comments.test.js
  comments
    ✕ returns all comments for a single post from the database (5 ms)
    ✕ creates a new comment (6 ms)
    ✕ allows admins and moderators to delete a comment (4 ms)
    ✕ does not allow a non-moderator to delete a comment (4 ms)
    ✕ does not allow a logged out user to delete a comment (5 ms)

  ● comments › returns all comments for a single post from the database

    TypeError: Cannot redefine property: __RESOLVED_TMP_DIR__
        at Function.defineProperty (<anonymous>)

      at Object.get (node_modules/jest-environment-node/build/index.js:100:20)
      at Object.<anonymous> (node_modules/temp-write/node_modules/temp-dir/index.js:7:12)
      at Object.<anonymous> (node_modules/temp-write/index.js:8:17)
      at Object.<anonymous> (node_modules/@prisma/internals/dist/engine-commands/queryEngineCommons.js:44:33)
      at Object.<anonymous> (node_modules/@prisma/internals/dist/engine-commands/getConfig.js:43:33)
      at Object.<anonymous> (node_modules/@prisma/internals/dist/engine-commands/index.js:29:24)
      at Object.<anonymous> (node_modules/@prisma/internals/dist/index.js:120:25)
      at configureTeardown (node_modules/@redwoodjs/testing/config/jest/api/jest.setup.js:35:23)
      at Object.<anonymous> (node_modules/@redwoodjs/testing/config/jest/api/jest.setup.js:225:11)

  ● comments › returns all comments for a single post from the database

    TypeError: Cannot redefine property: __RESOLVED_TMP_DIR__
        at Function.defineProperty (<anonymous>)

      at Object.get (node_modules/jest-environment-node/build/index.js:100:20)
      at Object.<anonymous> (node_modules/temp-write/node_modules/temp-dir/index.js:7:12)
      at Object.<anonymous> (node_modules/temp-write/index.js:8:17)
      at Object.<anonymous> (node_modules/@prisma/internals/dist/engine-commands/queryEngineCommons.js:44:33)
      at Object.<anonymous> (node_modules/@prisma/internals/dist/engine-commands/getConfig.js:43:33)
      at Object.<anonymous> (node_modules/@prisma/internals/dist/engine-commands/index.js:29:24)
      at Object.<anonymous> (node_modules/@prisma/internals/dist/index.js:120:25)
      at getQuoteStyle (node_modules/@redwoodjs/testing/config/jest/api/jest.setup.js:60:42)
      at teardown (node_modules/@redwoodjs/testing/config/jest/api/jest.setup.js:139:28)
      at Object.<anonymous> (node_modules/@redwoodjs/testing/config/jest/api/jest.setup.js:237:11)
...

@virtuoushub
Copy link
Collaborator

virtuoushub commented Nov 6, 2022

fwiw, I think I can get around the errors I am seeing using try/catch (plz ignore formatting diffs due to ;):
image

redwoodjs/example-storybook#196

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix This PR is a fix
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants