Eppo-exp / node-server-sdk

Eppo Node.js SDK
MIT License
9 stars 0 forks source link

upgrade jest to 29.7.0 #43

Closed leoromanovsky closed 10 months ago

leoromanovsky commented 10 months ago

Fixes: #issue

Motivation and Context

same as https://github.com/Eppo-exp/js-client-sdk/pull/40

Screenshot 2023-10-19 at 12 24 23 AM

Description

➜  node-server-sdk git:(lr/upgrade-security) ✗ yarn why semver
yarn why v1.22.19
[1/4] 🤔  Why do we have the module "semver"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "semver@7.5.4"
info Has been hoisted to "semver"
info Reasons this module exists
   - Hoisted from "ts-jest#semver"
   - Hoisted from "eslint#semver"
   - Hoisted from "@typescript-eslint#eslint-plugin#semver"
   - Hoisted from "@microsoft#api-extractor#semver"
   - Hoisted from "@typescript-eslint#parser#@typescript-eslint#typescript-estree#semver"
   - Hoisted from "@microsoft#api-documenter#@rushstack#node-core-library#semver"
   - Hoisted from "jest#@jest#core#jest-snapshot#semver"
   - Hoisted from "jest#@jest#core#@jest#reporters#istanbul-lib-instrument#semver"
info Disk size without dependencies: "308KB"
info Disk size with unique dependencies: "992KB"
info Disk size with transitive dependencies: "992KB"
info Number of shared dependencies: 1
=> Found "@babel/core#semver@6.3.1"
info This module exists because "@babel#core" depends on it.
info Disk size without dependencies: "88KB"
info Disk size with unique dependencies: "88KB"
info Disk size with transitive dependencies: "88KB"
info Number of shared dependencies: 0
=> Found "@babel/helper-compilation-targets#semver@6.3.1"
info This module exists because "@babel#core#@babel#helper-compilation-targets" depends on it.
info Disk size without dependencies: "88KB"
info Disk size with unique dependencies: "88KB"
info Disk size with transitive dependencies: "88KB"
info Number of shared dependencies: 0
=> Found "make-dir#semver@6.3.1"
info This module exists because "jest#@jest#core#@jest#reporters#istanbul-lib-report#make-dir" depends on it.
info Disk size without dependencies: "88KB"
info Disk size with unique dependencies: "88KB"
info Disk size with transitive dependencies: "88KB"
info Number of shared dependencies: 0
=> Found "istanbul-lib-instrument#@babel/core#semver@6.3.1"
info This module exists because "jest#@jest#core#@jest#reporters#istanbul-lib-instrument#@babel#core" depends on it.
info Disk size without dependencies: "88KB"
info Disk size with unique dependencies: "88KB"
info Disk size with transitive dependencies: "88KB"
info Number of shared dependencies: 0
=> Found "babel-plugin-istanbul#semver@6.3.1"
info Reasons this module exists
   - "jest#@jest#core#@jest#transform#babel-plugin-istanbul#istanbul-lib-instrument" depends on it
   - Hoisted from "jest#@jest#core#@jest#transform#babel-plugin-istanbul#istanbul-lib-instrument#semver"
   - Hoisted from "jest#@jest#core#@jest#transform#babel-plugin-istanbul#istanbul-lib-instrument#@babel#core#semver"
   - Hoisted from "jest#@jest#core#@jest#transform#babel-plugin-istanbul#istanbul-lib-instrument#@babel#core#@babel#helper-compilation-targets#semver"
info Disk size without dependencies: "88KB"
info Disk size with unique dependencies: "88KB"
info Disk size with transitive dependencies: "88KB"
info Number of shared dependencies: 0
=> Found "istanbul-lib-instrument#@babel/helper-compilation-targets#semver@6.3.1"
info This module exists because "jest#@jest#core#@jest#reporters#istanbul-lib-instrument#@babel#core#@babel#helper-compilation-targets" depends on it.
info Disk size without dependencies: "88KB"
info Disk size with unique dependencies: "88KB"
info Disk size with transitive dependencies: "88KB"
info Number of shared dependencies: 0
✨  Done in 0.19s.

How has this been tested?

leoromanovsky commented 10 months ago

Sorry marked for review but just noticed that tests are failing. After upgrading ts-jest locally got a warning that jest 29 is not yet supported by it:

➜  node-server-sdk git:(lr/upgrade-security) ✗ yarn test
yarn run v1.22.19
$ yarn test:unit
$ NODE_ENV=test jest '.*\.spec\.ts'
Determining test suites to run...ts-jest[versions] (WARN) Version 29.7.0 of jest installed has not been tested with ts-jest. If you're experiencing issues, consider using a supported version (>=27.0.0 <28.0.0-0). Please do not report issues in ts-jest if you are using unsupported versions.
Error: Jest: Got error running globalSetup - /Users/leo/src/node-server-sdk/test/globalSetup.ts, reason: ● Invalid return value:
  `process()` or/and `processAsync()` method of code transformer found at
  "/Users/leo/src/node-server-sdk/node_modules/ts-jest/dist/index.js"
  should return an object or a Promise resolving to an object. The object
  must have `code` property with a string of processed code.
  This error may be caused by a breaking change in Jest 28:
  https://jestjs.io/docs/28.x/upgrading-to-jest28#transformer
  Code Transformation Documentation:
  https://jestjs.io/docs/code-transformation

    at ScriptTransformer._buildTransformResult (/Users/leo/src/node-server-sdk/node_modules/@jest/transform/build/ScriptTransformer.js:442:15)
    at ScriptTransformer.transformSource (/Users/leo/src/node-server-sdk/node_modules/@jest/transform/build/ScriptTransformer.js:554:17)
    at revertHook.exts (/Users/leo/src/node-server-sdk/node_modules/@jest/transform/build/ScriptTransformer.js:776:18)
    at Module._compile (/Users/leo/src/node-server-sdk/node_modules/pirates/lib/index.js:130:29)
    at Module._extensions..js (node:internal/modules/cjs/loader:1310:10)
    at Object.newLoader (/Users/leo/src/node-server-sdk/node_modules/pirates/lib/index.js:141:7)
    at Module.load (node:internal/modules/cjs/loader:1119:32)
    at Module._load (node:internal/modules/cjs/loader:960:12)
    at Module.require (node:internal/modules/cjs/loader:1143:19)
    at require (node:internal/modules/cjs/helpers:119:18)
error Command failed with exit code 1.
aarsilv commented 10 months ago

Interesting. At Team Engine we had:

"jest": "^29.5.0",
"@types/jest": "^29.2.4",
"ts-jest": "^29.0.0",

And it all seemed to work. Perhaps we need to install/update the types and/or ts-jest

leoromanovsky commented 10 months ago

After thinking about it for a few minutes I think we do want to run our common test case code against these SDKs, treating them as a black box basically. This will "double-test" things in commons, but will catch if we ever screw up wiring up commons to these.

I think we could do that by just rewriting the tests to return/compare the results from getXXXXAssignment()

Good idea - I think I will start these tests from scratch with that in mind. Will do a PR next week.