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.

Sign up for GitHub

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

Jump to bottom

INT-3349 - Upgrade Polly.JS #688

Merged
merged 2 commits into from Apr 18, 2022
Merged

INT-3349 - Upgrade Polly.JS #688

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a663060
INT-3349 - Upgrade Polly.JS
austinkelleher Apr 16, 2022
a86b396
Upgrade to `fs-extra@^10.1.0` that fixes regressions
austinkelleher Apr 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 4 additions & 7 deletions packages/integration-sdk-cli/package.json
View file
Open in desktop
Expand Up @@ -25,7 +25,7 @@
"@jupiterone/integration-sdk-runtime": "^8.11.0",
"chalk": "^4",
"commander": "^5.0.0",
"fs-extra": "^10.0.0",
"fs-extra": "^10.1.0",
"globby": "^11.0.0",
"js-yaml": "^4.1.0",
"json-diff": "^0.5.4",
Expand All @@ -38,16 +38,13 @@
},
"devDependencies": {
"@jupiterone/integration-sdk-private-test-utils": "^8.11.0",
"@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",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

"@pollyjs/core": "^6.0.5",
"@pollyjs/persister-fs": "^6.0.5",
"@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",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See description. These are no longer needed! 🥳

"@types/pollyjs__core": "^4.3.3",
"@types/pollyjs__persister": "^4.3.1",
"@types/vis": "^4.21.20",
"memfs": "^3.2.0",
"neo-forgery": "^2.0.0",
Expand Down
10 changes: 2 additions & 8 deletions packages/integration-sdk-cli/src/__tests__/cli-run-failure.test.ts
View file
Open in desktop
Expand Up @@ -20,6 +20,7 @@ import { createCli } from '../index';
import { setupSynchronizerApi } from './util/synchronization';

import * as log from '../log';
import { createTestPolly } from './util/recording';

jest.mock('../log');

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

polly = new Polly('run-cli-failure', {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

adapters: ['node-http'],
persister: 'fs',
logging: false,
matchRequestsBy: {
headers: false,
},
});
polly = createTestPolly('run-cli-failure');

mocked(mockCreateIntegrationLogger).mockReturnValue(
createIntegrationLogger({ name: 'test' }),
Expand Down
11 changes: 2 additions & 9 deletions packages/integration-sdk-cli/src/__tests__/cli-run.test.ts
View file
Open in desktop
Expand Up @@ -16,6 +16,7 @@ import {
} from '@jupiterone/integration-sdk-core';

import * as log from '../log';
import { createTestPolly } from './util/recording';

jest.mock('../log');

Expand All @@ -28,15 +29,7 @@ beforeEach(() => {
process.env.JUPITERONE_API_KEY = 'testing-key';
process.env.JUPITERONE_ACCOUNT = 'mochi';

polly = new Polly('run-cli', {
adapters: ['node-http'],
persister: 'fs',
logging: false,
matchRequestsBy: {
headers: false,
},
});

polly = createTestPolly('run-cli');
loadProjectStructure('typeScriptIntegrationProject');

jest.spyOn(process, 'exit').mockImplementation((code: number | undefined) => {
Expand Down
11 changes: 2 additions & 9 deletions packages/integration-sdk-cli/src/__tests__/cli-sync.test.ts
View file
Open in desktop
Expand Up @@ -13,6 +13,7 @@ import {
} from './util/synchronization';

import * as log from '../log';
import { createTestPolly } from './util/recording';

jest.mock('../log');

Expand All @@ -25,15 +26,7 @@ beforeEach(() => {
process.env.JUPITERONE_API_KEY = 'testing-key';
process.env.JUPITERONE_ACCOUNT = 'mochi';

polly = new Polly('sync-cli', {
adapters: ['node-http'],
persister: 'fs',
logging: false,
matchRequestsBy: {
headers: false,
},
});

polly = createTestPolly('sync-cli');
loadProjectStructure('synchronization');

jest.spyOn(process, 'exit').mockImplementation((code: number | undefined) => {
Expand Down
16 changes: 16 additions & 0 deletions packages/integration-sdk-cli/src/__tests__/util/recording.ts
View file
Open in desktop
@@ -0,0 +1,16 @@
import { Polly, PollyConfig } from '@pollyjs/core';

export function createTestPolly(
recordingName: string,
pollyConfigOverrides?: PollyConfig,
) {
return new Polly(recordingName, {
adapters: ['node-http'],
persister: 'fs',
logLevel: 'silent',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

matchRequestsBy: {
headers: false,
},
...pollyConfigOverrides,
});
}
6 changes: 4 additions & 2 deletions packages/integration-sdk-cli/src/__tests__/util/synchronization.ts
View file
Open in desktop
Expand Up @@ -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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

res.status(200).json({ job });
});

Expand All @@ -37,7 +37,9 @@ export function setupSynchronizerApi({ polly, job, baseUrl }: SetupOptions) {
.post(`${baseUrl}/persister/synchronization/jobs/${job.id}/relationships`)
.intercept((req, res) => {
allowCrossOrigin(req, res);
job.numRelationshipsUploaded += JSON.parse(req.body).relationships.length;
job.numRelationshipsUploaded += JSON.parse(
req.body!,
).relationships.length;
res.status(200).json({ job });
});

Expand Down
10 changes: 3 additions & 7 deletions packages/integration-sdk-testing/package.json
View file
Open in desktop
Expand Up @@ -25,13 +25,9 @@
"dependencies": {
"@jupiterone/integration-sdk-core": "^8.11.0",
"@jupiterone/integration-sdk-runtime": "^8.11.0",
"@pollyjs/adapter-node-http": "^5.1.1",
"@pollyjs/core": "^5.1.1",
"@pollyjs/persister-fs": "^5.1.1",
"@types/har-format": "^1.2.4",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

"@types/pollyjs__adapter-node-http": "^2.0.1",
"@types/pollyjs__core": "^4.3.3",
"@types/pollyjs__persister": "^4.3.1",
"@pollyjs/adapter-node-http": "^6.0.5",
"@pollyjs/core": "^6.0.5",
"@pollyjs/persister-fs": "^6.0.5",
"deepmerge": "^4.2.2",
"lodash": "^4.17.15"
},
Expand Down
8 changes: 4 additions & 4 deletions packages/integration-sdk-testing/src/recording.ts
View file
Open in desktop
@@ -1,16 +1,16 @@
import { Har, Entry } from 'har-format';
import defaultsDeep from 'lodash/defaultsDeep';
import { gunzipSync } from 'zlib';

import NodeHttpAdapter from '@pollyjs/adapter-node-http';
import { Polly, PollyConfig } from '@pollyjs/core';
import FSPersister from '@pollyjs/persister-fs';
import { HarEntry, Har } from '@pollyjs/persister';

Polly.register(NodeHttpAdapter);
Polly.register(FSPersister);

export type Recording = Polly;
export type RecordingEntry = Entry;
export type RecordingEntry = HarEntry;

export interface SetupRecordingInput {
directory: string;
Expand Down Expand Up @@ -78,7 +78,7 @@ export function setupRecording({
return 'JupiterOneIntegationFSPersister';
}

saveRecording(recordingId: number, data: Har): void {
async onSaveRecording(recordingId: string, data: Har): Promise<void> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

data.log.entries.forEach((entry) => {
// Redact tokens, even though they expire
entry.request.headers = entry.request.headers.map((header) => ({
Expand All @@ -104,7 +104,7 @@ export function setupRecording({
mutateEntry?.(entry);
});

super.saveRecording(recordingId, data);
return super.onSaveRecording(recordingId, data);
}
}

Expand Down