Skip to content

[APITEST-766] Added Packages to Collection Schema #1352

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

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

[APITEST-766] Added Packages to Collection Schema #1352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f18fa6b
[APITEST-766] Added Packages to Collection Schema
aditya-baradwaj Feb 12, 2024
d4447f2
[APITEST-766] Fixed typos and minor issues
aditya-baradwaj Feb 12, 2024
d84f1ee
[APITEST-766] Added test changes for packages
aditya-baradwaj Feb 15, 2024
ffa818c
[APITEST-766] Added minor formatting fixes
aditya-baradwaj Feb 15, 2024
805b014
[APITEST-766] Changed packages to an object
aditya-baradwaj Feb 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions examples/collection-v2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,16 @@
"id": "my-global-script-1",
"script": {
"type": "text/javascript",
"exec": "console.log(\"hello\");"
"exec": "console.log(\"hello\");",
"packages": { "package1": {"id":"script-package-1"}}
}
},
{
"listen": "prerequest",
"script": {
"type": "text/javascript",
"exec": "console.log(\"hello\");"
"exec": "console.log(\"hello\");",
"packages": { "package1": {"id":"script-package-1"}}
}
}
],
Expand Down
4 changes: 4 additions & 0 deletions lib/collection/script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ _.assign(Script.prototype, /** @lends Script.prototype */ {
* @param {String} [options.type] Script type
* @param {String} [options.src] Script source url
* @param {String[]|String} [options.exec] Script to execute
* @param {Object} [options.packages] Packages required by the script
*/
update: function (options) {
// no splitting is being done here, as string scripts are split right before assignment below anyway
Expand All @@ -62,6 +63,9 @@ _.assign(Script.prototype, /** @lends Script.prototype */ {
* @type {string}
*/
this.type = options.type || 'text/javascript';

this.packages = options.packages || {};
Copy link
Member

Choose a reason for hiding this comment

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

Let's keep the @type properties for packages. Needs to be done for line 52 as well.

Copy link
Member

Choose a reason for hiding this comment

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

Do we need to add a default value if this key is not present? If it's an optional property, I'm guessing... no?

Copy link
Author

Choose a reason for hiding this comment

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

@coditva Do we mean to make this completely optional?
For consistency purposes, I thought it would be best to have this property in all event objects, and rather than leaving it as undefined in the cases where it isn't available, it seemed better to have it as an empty object.

We can change this in such a way that we move this under an
if (options.packages) to make this completely optional, but it does not seem ideal IMO. Do let me know if this is not the case.

It would be completely optional from a User perspective in either way tho.

Copy link
Member

Choose a reason for hiding this comment

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

A whole lot of collections will start having empty packages field. I want to avoid it for the collections that don't even use it.

Copy link
Author

Choose a reason for hiding this comment

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

makes sense, I'll make it so that this.packages is only set if options.packages exists. Sounds good?


_.has(options, 'src') && (

/**
Expand Down
12 changes: 8 additions & 4 deletions test/unit/collection.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ describe('Collection', function () {
script: {
id: 'my-script-1',
type: 'text/javascript',
exec: ['console.log("This doesn\'t matter");']
exec: ['console.log("This doesn\'t matter");'],
packages: {}
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to add the empty key everywhere? It's supposed to be optional, no?

Copy link
Author

Choose a reason for hiding this comment

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

Yep yep, just needed to add this to the assertions after conversion since we're defaulting to an empty object, no need for this here.

}
}]
},
Expand Down Expand Up @@ -178,7 +179,8 @@ describe('Collection', function () {
script: {
id: 'test-script-1',
type: 'text/javascript',
exec: ['console.log("Random");']
exec: ['console.log("Random");'],
packages: {}
}
});
expect(collectionJSON.event[1]).to.have.property('listen', 'prerequest');
Expand Down Expand Up @@ -218,7 +220,8 @@ describe('Collection', function () {
type: 'text/javascript',
exec: [
'console.log("bcoz I am batman!");'
]
],
packages: { superman: { id: 'script-apckage-1' } }
}
}],
item: [{
Expand Down Expand Up @@ -280,7 +283,8 @@ describe('Collection', function () {
type: 'text/javascript',
exec: [
'console.log("bcoz I am batman!");'
]
],
packages: { superman: { id: 'script-apckage-1' } }
}
}],
item: [{
Expand Down
12 changes: 8 additions & 4 deletions test/unit/event-list.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ describe('EventList', function () {
id: 'my-global-script-1',
script: {
type: 'text/javascript',
exec: 'console.log("hello");'
exec: 'console.log("hello");',
packages: [{ id: 'script-package-1', name: 'package1' }]
Copy link
Member

Choose a reason for hiding this comment

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

Needs to be updated?

}
}];

Expand All @@ -35,7 +36,8 @@ describe('EventList', function () {
script: {
id: 'test-script-1',
type: 'text/javascript',
exec: 'console.log("hello");'
exec: 'console.log("hello");',
packages: { package1: { id: 'script-package-1' } }
}
}]),
eventListJSON;
Expand All @@ -46,7 +48,8 @@ describe('EventList', function () {
script: {
id: 'test-script-1',
type: 'text/javascript',
exec: ['console.log("hello");']
exec: ['console.log("hello");'],
packages: { package1: { id: 'script-apckage-1' } }
}
}]);

Expand All @@ -63,7 +66,8 @@ describe('EventList', function () {
script: {
id: 'test-script-1',
type: 'text/javascript',
exec: ['console.log("hello");']
exec: ['console.log("hello");'],
packages: { package1: { id: 'script-apckage-1' } }
}
});

Expand Down
10 changes: 7 additions & 3 deletions test/unit/event.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ describe('Event', function () {
id: 'my-global-script-1',
script: {
type: 'text/javascript',
exec: 'console.log("hello");'
exec: 'console.log("hello");',
packages: {}
}
};

Expand Down Expand Up @@ -52,6 +53,7 @@ describe('Event', function () {
expect(postmanEvent).to.have.property('script').that.is.an('object');
expect(postmanEvent.script).to.have.property('type', 'text/javascript');
expect(postmanEvent.script).to.have.property('exec').that.is.an('array');
expect(postmanEvent.script).to.have.property('packages').that.is.an('object');
});
});

Expand Down Expand Up @@ -120,7 +122,8 @@ describe('Event', function () {
expect(eventJSON).to.have.property('script').that.has.property('id');
expect(eventJSON.script).to.deep.include({
type: 'text/javascript',
exec: ['console.log("hello");']
exec: ['console.log("hello");'],
packages: {}
});
});

Expand All @@ -132,7 +135,8 @@ describe('Event', function () {
expect(beforeJSON).to.have.property('script').that.has.property('id');
expect(beforeJSON.script).to.deep.include({
type: 'text/javascript',
exec: ['console.log("hello");']
exec: ['console.log("hello");'],
packages: {}
});

event.update({ script: { id: 'my-new-script' } });
Expand Down
6 changes: 4 additions & 2 deletions test/unit/item-group.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ describe('ItemGroup', function () {
script: {
id: 'my-script-1',
type: 'text/javascript',
exec: ['console.log("This doesn\'t matter");']
exec: ['console.log("This doesn\'t matter");'],
packages: { package1: { id: 'script-apckage-1' } }
}
}],
protocolProfileBehavior: {
Expand Down Expand Up @@ -527,7 +528,8 @@ describe('ItemGroup', function () {
script: {
id: 'my-test-script',
type: 'text/javascript',
exec: ['console.log("hello there!");']
exec: ['console.log("hello there!");'],
packages: { package1: { id: 'script-apckage-1' } }
}
}],
item: [{
Expand Down
3 changes: 2 additions & 1 deletion test/unit/item.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ describe('Item', function () {
script: {
id: 'my-script-1',
type: 'text/javascript',
exec: ['console.log("This doesn\'t matter");']
exec: ['console.log("This doesn\'t matter");'],
packages: { package1: { id: 'script-apckage-1' } }
}
}],
request: {
Expand Down
6 changes: 4 additions & 2 deletions test/unit/script.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ describe('Script', function () {
expect(scriptJSON).to.have.property('id');
expect(scriptJSON).to.deep.include({
type: 'text/javascript',
exec: ['console.log("This is a line of test script code");']
exec: ['console.log("This is a line of test script code");'],
packages: {}
});
});

Expand Down Expand Up @@ -201,7 +202,8 @@ describe('Script', function () {

expect(jsonified).to.deep.include({
type: rawScript.type,
exec: rawScript.exec.split('\n')
exec: rawScript.exec.split('\n'),
packages: rawScript.packages
});
expect(jsonified.src).to.eql(rawScript.src);
});
Expand Down
3 changes: 3 additions & 0 deletions types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2158,15 +2158,18 @@ declare module "postman-collection" {
* @param [options.type] - Script type
* @param [options.src] - Script source url
* @param [options.exec] - Script to execute
* @param [options.packages] - packages to be preloaded
*/
update(options?: {
type?: string;
src?: string;
exec?: string[] | string;
packages?: Object;
}): void;
type: string;
src: Url;
exec: string[];
packages: Object;
/**
* Check whether an object is an instance of ItemGroup.
* @param obj - -
Expand Down