INT-3349 - Upgrade Polly.JS #688

austinkelleher · 2022-04-16T13:33:17Z

The following packages have been upgraded:

@pollyjs/adapter-node-http
@pollyjs/core
@pollyjs/persister-fs

The new Polly.JS packages ship with TypeScript definition files,
so the old @type/pollyjs__ packages have been removed!

Additionally, fs-extra has been pinned to semver range ^9.0.0
because there a change was released in ^10.0.0 that caused
a regression. See: jprichardson/node-fs-extra#953.

NOTE: I also linked the @jupiterone/integration-sdk-testing package into an integration and manually tested the following scenarios:

Run tests with existing recordings
Delete a recording, generate a new recording, and test against the new recording

austinkelleher · 2022-04-16T13:33:35Z

package.json

@@ -35,5 +35,8 @@
    "ts-jest": "^27.1.0",
    "typescript": "^4.2.4"
  },
+  "resolutions": {
+    "fs-extra": "^9.0.0"


See description for the reason for this

austinkelleher · 2022-04-16T13:34:14Z

packages/integration-sdk-cli/package.json

-    "@pollyjs/adapter-node-http": "^5.1.1",
-    "@pollyjs/core": "^5.1.1",
-    "@pollyjs/persister-fs": "^5.1.1",
+    "@pollyjs/adapter-node-http": "^6.0.5",


Major version changes were considered and various testing utilities have been updated. CHANGELOG can be found here: https://github.com/Netflix/pollyjs/blob/master/CHANGELOG.md#600-2021-11-30

austinkelleher · 2022-04-16T13:34:26Z

packages/integration-sdk-cli/package.json

    "@types/fs-extra": "^9.0.13",
    "@types/js-yaml": "^4.0.3",
    "@types/json-diff": "^0.5.1",
    "@types/lodash": "^4.14.158",
-    "@types/pollyjs__adapter-node-http": "^2.0.1",


See description. These are no longer needed! 🥳

austinkelleher · 2022-04-16T13:34:51Z

packages/integration-sdk-cli/src/__tests__/cli-run-failure.test.ts

@@ -37,14 +38,7 @@ beforeEach(() => {
  process.env.JUPITERONE_ACCOUNT = 'mochi';
  loadProjectStructure('validationFailure');

-  polly = new Polly('run-cli-failure', {


Small bonus change. Extracted this functionality into createTestPolly because it is used in 3 tests.

austinkelleher · 2022-04-16T13:35:58Z

packages/integration-sdk-cli/src/__tests__/util/recording.ts

+  return new Polly(recordingName, {
+    adapters: ['node-http'],
+    persister: 'fs',
+    logLevel: 'silent',


The only change made when creating new Polly instances is here. There used to be a logging boolean, which was assigned false. In Polly.JS v6, logging was removed and replaced with logLevel. I kept the existing behavior by assigning silent as the value.

austinkelleher · 2022-04-16T13:36:50Z

packages/integration-sdk-cli/src/__tests__/util/synchronization.ts

@@ -22,7 +22,7 @@ export function setupSynchronizerApi({ polly, job, baseUrl }: SetupOptions) {
    .post(`${baseUrl}/persister/synchronization/jobs/${job.id}/entities`)
    .intercept((req, res) => {
      allowCrossOrigin(req, res);
-      job.numEntitiesUploaded += JSON.parse(req.body).entities.length;
+      job.numEntitiesUploaded += JSON.parse(req.body!).entities.length;


The new internal Polly.JS TypeScript definitions allow body to be undefined. For this test to pass, body would need to be defined.

austinkelleher · 2022-04-16T13:38:52Z

packages/integration-sdk-testing/src/recording.ts

@@ -78,7 +78,7 @@ export function setupRecording({
      return 'JupiterOneIntegationFSPersister';
    }

-    saveRecording(recordingId: number, data: Har): void {
+    async onSaveRecording(recordingId: string, data: Har): Promise<void> {


Breaking change in Polly.JS v6. See the Polly.JS CHANGELOG.

The following packages have been upgraded: - `@pollyjs/adapter-node-http` - `@pollyjs/core` - `@pollyjs/persister-fs` The new Polly.JS packages ship with TypeScript definition files, so the old `@type/pollyjs__` packages have been removed! Additionally, `fs-extra` has been pinned to semver range `^9.0.0` because there a change was released in `^10.0.0` that caused a regression. See: jprichardson/node-fs-extra#953.

austinkelleher · 2022-04-16T13:41:16Z

packages/integration-sdk-testing/package.json

-    "@pollyjs/adapter-node-http": "^5.1.1",
-    "@pollyjs/core": "^5.1.1",
-    "@pollyjs/persister-fs": "^5.1.1",
-    "@types/har-format": "^1.2.4",


No longer needed since this data is exported from @pollyjs/persister! 🥳

lamweili · 2022-04-17T16:52:16Z

package.json

@@ -35,5 +35,8 @@
    "ts-jest": "^27.1.0",
    "typescript": "^4.2.4"
  },
+  "resolutions": {
+    "fs-extra": "^9.0.0"


Related to fs.realpath.native being undefined, causing import of fs-extra@10.0.0 to throw error.

Defensively patched in jprichardson/node-fs-extra#953 and released in fs-extra@10.1.0.

Might want to bump it up instead, maybe?

But still, why is fs.realpath.native undefined? That is a root cause to be investigated.

@peteriman Thanks a lot for putting a fix in for this issue and for following up here!

It's not clear to me why fs.realpath.native is undefined either. I may spend a little time trying to figure that out and get back to you if I find anything.

Thanks again!

While fs-extra no longer throws error on require/import, if someone still uses fs.realpath.native (be it from fs-extra or from native fs) they would still face errors.

@austinkelleher You might want to console.log(typeof fs.realpath.native) at the first line of code as a pre-requisite, then the subsequently before and after every require/import to identify the offending dependency that "deleted" it.

austinkelleher commented Apr 16, 2022

View reviewed changes

austinkelleher force-pushed the INT-3349-polly branch from c4577e9 to a663060 Compare April 16, 2022 13:40

austinkelleher commented Apr 16, 2022

View reviewed changes

austinkelleher marked this pull request as ready for review April 16, 2022 13:47

austinkelleher requested a review from a team April 16, 2022 13:47

lamweili suggested changes Apr 17, 2022

View reviewed changes

Upgrade to fs-extra@^10.1.0 that fixes regressions

a86b396

adam-in-ict approved these changes Apr 18, 2022

View reviewed changes

austinkelleher merged commit cdd6741 into main Apr 18, 2022

austinkelleher deleted the INT-3349-polly branch April 18, 2022 20:35

austinkelleher mentioned this pull request Apr 20, 2022

v8.12.0 #692

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

INT-3349 - Upgrade Polly.JS #688

INT-3349 - Upgrade Polly.JS #688

austinkelleher commented Apr 16, 2022 •

edited

austinkelleher Apr 16, 2022

austinkelleher Apr 16, 2022

austinkelleher Apr 16, 2022

austinkelleher Apr 16, 2022

austinkelleher Apr 16, 2022

austinkelleher Apr 16, 2022

austinkelleher Apr 16, 2022

austinkelleher Apr 16, 2022

lamweili Apr 17, 2022

austinkelleher Apr 18, 2022

lamweili Apr 19, 2022 •

edited

INT-3349 - Upgrade Polly.JS #688

INT-3349 - Upgrade Polly.JS #688

Conversation

austinkelleher commented Apr 16, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lamweili Apr 19, 2022 • edited

Choose a reason for hiding this comment

austinkelleher commented Apr 16, 2022 •

edited

lamweili Apr 19, 2022 •

edited