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

Added flag to set verbose output size limit for newman run #2883

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
10 changes: 7 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,9 @@ For more details on [Reporters](#reporters) and writing your own [External Repor
- `--timeout-script <ms>`<br />
Specify the time (in milliseconds) to wait for scripts to complete execution.

- `--output-size <KB>`<br />
Copy link
Member

Choose a reason for hiding this comment

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

As we all know, naming is the hardest part of the equation. This name has some way to go to resonate with what it does - "specify the truncation point of a body in verbose CLI output for requests and responses."

How about bracketing it under "reporter-cli-*" params?

Copy link
Author

Choose a reason for hiding this comment

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

Having gone through the reporter-cli params, I think bracketing it under reporter-cli-* would be more appropriate
How about replacing the current output-size flag with two reporter-cli flags

  • reporter-cli-verbose-output-limit <Size in any memory unit>
  • reporter-cli-no-verbose-output limit : Sets the limit to Infinite

It's rare to see Command Line tools allow users to pass arguments as infinite. Plus spelling errors might creep in.Mostly, they achieve by simply setting some flag.

Copy link
Author

Choose a reason for hiding this comment

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

Hey @shamasis, any thoughts regarding this?

Specify the output size limit ( in KB ) when verbose flag is set. Specify `infinite` to remove any output size limit.

- `-k`, `--insecure`<br />
Disables SSL verification checks and allows self-signed SSL certificates.

Expand Down Expand Up @@ -288,6 +291,7 @@ return of the `newman.run` function is a run instance, which emits run events th
| options.timeout | Specify the time (in milliseconds) to wait for the entire collection run to complete execution.<br /><br />_Optional_<br />Type: `number`, Default value: `Infinity` |
| options.timeoutRequest | Specify the time (in milliseconds) to wait for requests to return a response.<br /><br />_Optional_<br />Type: `number`, Default value: `Infinity` |
| options.timeoutScript | Specify the time (in milliseconds) to wait for scripts to return a response.<br /><br />_Optional_<br />Type: `number`, Default value: `Infinity` |
| options.outputSize | Specify the output size limit ( in KB ) when verbose flag is set.<br /><br />For example, `outputSize = 2` sets the verbose output limit to 2KB or 2048 bytes.<br /><br />To remove any verbose output limit, pass `infinite` to `--output-size`. This would set `outputSize` to `Number. POSITIVE_INFINITY`<br /><br />_Optional_<br />Type: `number`, Default value: `2` |
Copy link
Member

@shamasis shamasis Dec 8, 2021

Choose a reason for hiding this comment

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

Is KB the right unit? Any other option around size that we can draw pattern from?

Any way we can take more dynamic input by parsing units? I'm sure there will be some popular library that can take "10kb", "1Mb" as inputs and spit out absolute bytes. 😛

Copy link
Author

Choose a reason for hiding this comment

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

Sure, will implement this. In case of invalid inputs, should we proceed with default 2KB or throw an error?

| options.delayRequest | Specify the time (in milliseconds) to wait for between subsequent requests.<br /><br />_Optional_<br />Type: `number`, Default value: `0` |
| options.ignoreRedirects | This specifies whether newman would automatically follow 3xx responses from servers.<br /><br />_Optional_<br />Type: `boolean`, Default value: `false` |
| options.insecure | Disables SSL verification checks and allows self-signed SSL certificates.<br /><br />_Optional_<br />Type: `boolean`, Default value: `false` |
Expand Down Expand Up @@ -347,7 +351,7 @@ newman.run({
"_postman_exported_at": "2016-10-17T14:31:26.200Z",
"_postman_exported_using": "Postman/4.8.0"
},
globalVar: [
globalVar: [
{ "key":"glboalSecret", "value":"globalSecretValue" },
{ "key":"globalAnotherSecret", "value":`${process.env.GLOBAL_ANOTHER_SECRET}`}
],
Expand All @@ -367,7 +371,7 @@ newman.run({
"_postman_exported_at": "2016-10-17T14:26:34.940Z",
"_postman_exported_using": "Postman/4.8.0"
},
envVar: [
envVar: [
{ "key":"secret", "value":"secretValue" },
{ "key":"anotherSecret", "value":`${process.env.ANOTHER_SECRET}`}
],
Expand Down Expand Up @@ -446,7 +450,7 @@ such a scenario.
|-------------|-------------------|
| `--reporter-cli-silent` | The CLI reporter is internally disabled and you see no output to terminal. |

| `--reporter-cli-show-timestamps` | This prints the local time for each request made. |
| `--reporter-cli-show-timestamps` | This prints the local time for each request made. |
| `--reporter-cli-no-summary` | The statistical summary table is not shown. |
| `--reporter-cli-no-failures` | This prevents the run failures from being separately printed. |
| `--reporter-cli-no-assertions` | This turns off the output for request-wise assertions as they happen. |
Expand Down
1 change: 1 addition & 0 deletions bin/newman.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ program
.option('--timeout [n]', 'Specify a timeout for collection run (milliseconds)', util.cast.integer, 0)
.option('--timeout-request [n]', 'Specify a timeout for requests (milliseconds)', util.cast.integer, 0)
.option('--timeout-script [n]', 'Specify a timeout for scripts (milliseconds)', util.cast.integer, 0)
.option('--output-size [n]', 'Specify the output size limit for CLI ( in KB )', util.cast.integerOrInfinity, 2)
Copy link
Member

Choose a reason for hiding this comment

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

English here is ambiguous "output size of what? Entire CLI, body always, body in verbose"?!

Copy link
Author

Choose a reason for hiding this comment

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

Technically, this PR limits the verbose body output only. I'll make the necessary changes to remove this ambiguity.

.option('--working-dir <path>', 'Specify the path to the working directory')
.option('--no-insecure-file-read', 'Prevents reading the files situated outside of the working directory')
.option('-k, --insecure', 'Disables SSL validations')
Expand Down
16 changes: 16 additions & 0 deletions bin/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,22 @@ module.exports = {
return num.valueOf();
},

/**
* Helper to coerce number like strings into integers.
* If supplied argument is infinite, return positive infinity
* Perform safety checks, and return the result.
*
* @param {String} arg - The stringified number argument or infinite.
* @returns {Number} - The supplied argument, casted to an integer or positive infinity.
*/
integerOrInfinity: (arg) => {
Copy link
Member

Choose a reason for hiding this comment

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

How does Newman handle other properties that can have infinity? Any pattern to draw from there?

if (arg === 'infinite') {
return Number.POSITIVE_INFINITY;
}

return module.exports.cast.integer(arg);
},

/**
* Helper for collecting argument passed multiple times.
*
Expand Down
22 changes: 12 additions & 10 deletions lib/reporters/cli/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ var _ = require('lodash'),
process: 'process',
total: 'total'
},
BODY_CLIP_SIZE = 2048,
KB_TO_BYTE_MULTIPLIER = 1024,

PostmanCLIReporter,
timestamp,
Expand Down Expand Up @@ -233,7 +233,9 @@ PostmanCLIReporter = function (emitter, reporterOptions, options) {
`${mime.mimeType}`,
`${mime.mimeFormat}`,
`${mime.charset}`
].join(` ${colors.gray(symbols.star)} `);
].join(` ${colors.gray(symbols.star)} `),

bodyClipSize = options.outputSize * KB_TO_BYTE_MULTIPLIER;
Copy link
Member

Choose a reason for hiding this comment

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

Handle fallback for invalid bodySize in-line here



print.lf(SPC); // also flushes out the circling progress icon
Expand All @@ -252,10 +254,10 @@ PostmanCLIReporter = function (emitter, reporterOptions, options) {

// print request body
if (reqTextLen) {
// truncate very large request (is 2048 large enough?)
if (reqTextLen > BODY_CLIP_SIZE) {
reqText = reqText.substr(0, BODY_CLIP_SIZE) +
colors.brightWhite(`\n(showing ${util.filesize(BODY_CLIP_SIZE)}/${util.filesize(reqTextLen)})`);
// truncate very large request (depends on output-size flag)
if (reqTextLen > bodyClipSize) {
reqText = reqText.substr(0, bodyClipSize) +
colors.brightWhite(`\n(showing ${util.filesize(bodyClipSize)}/${util.filesize(reqTextLen)})`);
}

reqText = wrap(reqText, ` ${colors.white(symbols.console.middle)} `);
Expand All @@ -271,10 +273,10 @@ PostmanCLIReporter = function (emitter, reporterOptions, options) {

// print response body
if (resTextLen) {
// truncate very large response (is 2048 large enough?)
if (resTextLen > BODY_CLIP_SIZE) {
resText = resText.substr(0, BODY_CLIP_SIZE) +
colors.brightWhite(`\n(showing ${util.filesize(BODY_CLIP_SIZE)}/${util.filesize(resTextLen)})`);
// truncate very large response (depends on output-size flag)
if (resTextLen > bodyClipSize) {
resText = resText.substr(0, bodyClipSize) +
colors.brightWhite(`\n(showing ${util.filesize(bodyClipSize)}/${util.filesize(resTextLen)})`);
}

resText = wrap(resText, ` ${colors.white(symbols.console.middle)} `);
Expand Down
59 changes: 48 additions & 11 deletions test/cli/verbose.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,21 @@ var _ = require('lodash');

describe('newman run --verbose', function () {
var verboseStrings = [
'prepare',
'wait',
'dns-lookup',
'tcp-handshake',
'ssl-handshake',
'transfer-start',
'download',
'process',
'average DNS lookup time:',
'average first byte time:'
];
'prepare',
'wait',
'dns-lookup',
'tcp-handshake',
'ssl-handshake',
'transfer-start',
'download',
'process',
'average DNS lookup time:',
'average first byte time:'
],
endTags = [
'</body>',
'</html>'
];

it('should include verbose with --verbose option', function (done) {
exec('node ./bin/newman.js run test/fixtures/run/single-get-request.json --verbose', function (code, stdout) {
Expand All @@ -33,4 +37,37 @@ describe('newman run --verbose', function () {
done();
});
});

it('should limit to 2KB with --verbose option only without setting output-size', function (done) {
Copy link
Member

Choose a reason for hiding this comment

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

Very neat and creative test. Like it ❤️😊.

Another improvement could be if the body was autogenerated series of number marking increasing size. That way, we can both assert points and accuracy.

// eslint-disable-next-line max-len
exec('node ./bin/newman.js run test/fixtures/run/large-output-get-request.json --verbose', function (_code, stdout) {
_.forEach(endTags, function (str) {
expect(stdout).to.not.contain(str);
});

done();
});
});

it('should log the entire output --verbose option and --output-size set to infinte', function (done) {
// eslint-disable-next-line max-len
exec('node ./bin/newman.js run test/fixtures/run/large-output-get-request.json --verbose --output-size infinite', function (_code, stdout) {
_.forEach(endTags, function (str) {
expect(stdout).to.contain(str);
});

done();
});
});

it('should log more output when --output-size set to 4KB than 2KB', function (done) {
// eslint-disable-next-line max-len
exec('node ./bin/newman.js run test/fixtures/run/large-output-get-request.json --verbose --output-size 4', function (_code, largeStdout) {
// eslint-disable-next-line max-len
exec('node ./bin/newman.js run test/fixtures/run/large-output-get-request.json --verbose --output-size 2', function (_code, smallStdout) {
expect(largeStdout.length).to.be.greaterThan(smallStdout.length);
done();
});
});
});
});
18 changes: 18 additions & 0 deletions test/fixtures/run/large-output-get-request.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"info": {
"_postman_id": "6d0dfccd-5998-43bb-a0a5-f88065c8c5d1",
"name": "Large Output Test for --output-size Flag",
"schema": "https://schema.getpostman.com/json/collection/v2.0.0/collection.json"
},
"item": [
{
"name": "Postman Get Request",
"request": {
"method": "GET",
"header": [],
"url": "https://www.postman.com/"
},
"response": []
}
]
}
1 change: 1 addition & 0 deletions test/unit/cli.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ describe('cli parser', function () {
envVar: [],
folder: [],
insecureFileRead: true,
outputSize: 2,
color: 'auto',
timeout: 0,
timeoutRequest: 0,
Expand Down