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

Upload newman runs to Postman #2849

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
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
13 changes: 10 additions & 3 deletions bin/newman.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,15 @@ program
.option('--cookie-jar <path>', 'Specify the path to a custom cookie jar (serialized tough-cookie JSON) ')
.option('--export-cookie-jar <path>', 'Exports the cookie jar to a file after completing the run')
.option('--verbose', 'Show detailed information of collection run and each request sent')
.option('-p, --publish <workspace-id>', 'Publishes to given workspace')
.option('--publish-skip-response',
'Skip responses (headers, body, etc) while uploading newman run to Postman')
.option('--publish-retries', 'Number of times newman can try to publish before safely erroring out.',
util.cast.integer, 3)
.option('--publish-timeout', 'Timeout in seconds for uploading newman runs to postman',
util.cast.integer, 60)
.action((collection, command) => {
let options = util.commanderToObject(command),

// parse custom reporter options
reporterOptions = util.parseNestedOptions(program._originalArgs, '--reporter-', options.reporters);

Expand All @@ -74,14 +80,15 @@ program
acc[key] = _.assignIn(value, reporterOptions._generic); // overrides reporter options with _generic
}, {});

newman.run(options, function (err, summary) {
newman.run(options, function (err, summary, newmanStatus) {
const runError = err || summary.run.error || summary.run.failures.length;

if (err) {
console.error(`error: ${err.message || err}\n`);
err.friendly && console.error(` ${err.friendly}\n`);
}
runError && !_.get(options, 'suppressExitCode') && process.exit(1);

(runError || !newmanStatus.resultUploaded) && !_.get(options, 'suppressExitCode') && process.exit(1);
});
});

Expand Down
25 changes: 23 additions & 2 deletions lib/run/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ var _ = require('lodash'),
getOptions = require('./options'),
exportFile = require('./export-file'),
util = require('../util'),
print = require('../print'),
{ uploadRunToPostman, POSTMAN_UPLOAD_ERROR_LINK } = require('./upload-run'),

/**
* This object describes the various events raised by Newman, and what each event argument contains.
Expand Down Expand Up @@ -302,7 +304,10 @@ module.exports = function (options, callback) {
// in case runtime faced an error during run, we do not process any other event and emit `done`.
// we do it this way since, an error in `done` callback would have anyway skipped any intermediate
// events or callbacks
if (err) {

// TODO: Add analytics call here

if (err) { // TODO -> understand when this is raised
Copy link
Member

@coditva coditva Oct 6, 2021

Choose a reason for hiding this comment

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

@UmedJadhav This error is raised from the postman-runtime library if the collection run did not finish successfully.

emitter.emit('done', err, emitter.summary);
callback(err, emitter.summary);

Expand Down Expand Up @@ -343,7 +348,23 @@ module.exports = function (options, callback) {
asyncEach(emitter.exports, exportFile, function (err) {
// we now trigger actual done event which we had overridden
emitter.emit('done', err, emitter.summary);
callback(err, emitter.summary);
let publishSummary;

return uploadRunToPostman(emitter.summary, options)
.then((response) => {
publishSummary = { uploadSuccessful: true, resultUrl: response.message,
runPayloadSize: response.runPayloadSize };
print.lf(`Uploaded the newman run to postman.
Visit ${publishSummary.resultUrl} to view the results in postman web`);
})
.catch((error) => {
publishSummary = { uploadSuccessful: false, error: error.message };
print.lf(`Unable to upload the results to Postman:
Reason: ${error.message}
You can find solutions to common upload errors here: ${POSTMAN_UPLOAD_ERROR_LINK}`);
}).finally(() => {
callback(err, emitter.summary, publishSummary);
});
});
}
});
Expand Down
257 changes: 257 additions & 0 deletions lib/run/upload-run.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,257 @@
const _ = require('lodash'),
coditva marked this conversation as resolved.
Show resolved Hide resolved
sdk = require('postman-collection'),
util = require('../util'),
print = require('../print'),
retry = require('async-retry'),
request = require('postman-request'),

POSTMAN_UPLOAD_ERROR_LINK = '<TBA>',
UPLOAD_RUN_API_URL = '<TBA>',
NEWMAN_STRING = 'newman';

/**
* @private
* Internal upload call
*
* @param {Object} uploadOptions
* @returns {function} Returns an async function which can be used by async-retry library to have retries
*/
_upload = (uploadOptions) => {
Copy link
Member

Choose a reason for hiding this comment

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

If this is going to return a function, it should be called something like _getUploadRetryable.
Or, don't return a function! Just bind the params where you're using it.

return async (bail) => request.post(uploadOptions , (error, response, body) => {
if(error){
Copy link
Member

Choose a reason for hiding this comment

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

Indentation looks weird here.

if(error.code === 'ECONNREFUSED') { // Retry only if the ERROR is ERRCONNECT
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: 'ECONNREFUSED' should be a const.

throw new Error(error.message);
Copy link
Member

Choose a reason for hiding this comment

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

If the error message is same, why not just throw the error itself?

Suggested change
throw new Error(error.message);
throw error;

}
return bail(error); // For other errors , dont retry
}

// Handle exact status codes
Copy link
Member

Choose a reason for hiding this comment

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

We don't seem to be handling the exact status codes. 😅

Copy link
Member

Choose a reason for hiding this comment

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

Should use a switch-case in this case (pun intended.)


if(200 <= response.statusCode && response.statusCode <= 299){
return body;
}

if( 400 <= response.statusCode && response.statusCode<= 499 ){

if(response.statusCode === 404){
return bail(new Error('Couldn\'t find the postman server route'))
}

return bail(new Error(body.message)); // Avoid retry if there is client side error ( API key error / Workspace ID / permission error)

}

if( 500 <= response.statusCode && response.statusCode <= 599){ // Perform Retry if Server side Error
throw new Error(`Retrying server error: ${body.message}`); // Change this after discussion with Harsh
}

throw new Error(`Recieved an unexpected Response status code while uploading Newman run`); // This should not be activated ( Discuss with Harsh , how to handle 3xx )
});
}

/**
* @private
* Internal upload function which handles the retry
*
* @param {*} uploadOptions
* @param {*} retryOptions
*
* @returns {Promise}
*/
_uploadWithRetry = (upload, retryOptions) => {
return retry(upload,{
retries: retryOptions.maxRetries,
factor: retryOptions.retryDelayMultiplier,
randomize: retryOptions.addJitter || false,
maxRetryTime: retryOptions.maxRetryDelay * 1000 , // converting to ms
maxTimeout: retryOptions.totalTimeout * 1000, // converting to ms
onRetry: retryOptions.onRetry || function(){}
});
}

/**
* @private
*/
_buildRequestObject = (request)=> {

if (!request) {
return {};
}

request = new sdk.Request(request);

return {
url: _.invoke(request, 'url.toString', ''),
method: _.get(request, 'method', ''),
headers: request.getHeaders({ enabled: false }),
body: request.toJSON().body,
path: ''// '/ Collections / Create Collections / Create a collection into personal workspace' // TODO - Find out where is this used , ask giri if we can skip this ?
};

}

/**
* @private
* @param {Object} response
* @param {Boolean} skipResponse
*
* @returns {Object}
*/
_buildResponseObject = (response, skipResponse) => {

if(!response) {
return {}
}

return {
code: response.code,
name: response.status,
time: response.responseTime,
size: response.responseSize,
headers: response.header,
body: (!skipResponse && response.stream.data ) ? new TextDecoder('utf-8').decode(new Uint8Array(response.stream.data)) : null
}
}

/**
* @private
* @param {Array} assertions
*
* @returns {Array}
*/
_buildTestObject = (assertions) => {
const tests = []

assertions && assertions.forEach(assert => {

tests.push({
name: assert.assertion,
error: assert.error ? _.pick(assert.error, ['name','message','stack']) : null,
status: assert.error ? 'fail' : 'pass',
})
});

return tests
}

/**
* @private
* @param {Array} executions
* @param {Number} iterationCount
* @param {Boolean} skipResponse
*
* @returns {Array}
*/
_executionToIterationConverter = (executions, iterationCount, skipResponse) => {

const iterations = []
executions = util.partition(executions, iterationCount);

executions.forEach( iter => {
const iteration = []

iter.forEach(req => {
iteration.push({
id: req.item.id,
name: req.item.name || '',
request: _buildRequestObject(req.request),
response: req.response ? _buildResponseObject(req.response, skipResponse): null,
error: req.requestError || null,// @TODO - In RUnner, we are storing true for error / undefined if no error. Check for this with Harsh
tests: _buildTestObject(req.assertions), // What's the value if tests are not present for a request
});
});

iterations.push(iteration);

});
return iterations
}

/**
* @private
* @param {Object} runOptions
* @param {Object} runOptions
*
* @returns {String}
*/
_buildPostmanUploadPayload = (runOptions, runSummary) => {
if(!runOptions || !runSummary) {
throw new Error('Cannot Build Run Payload without runOptions or RunSummary');
}

const run = {
name: runOptions.collection.name || '',
status:'finished', // THis can be finished or failed - Check with Harsh about all the possible values
source: NEWMAN_STRING,
failedTestCount: runSummary.run.stats.assertions.failed || 0,
totalTestCount: runSummary.run.stats.assertions.total || 0,
collection: runOptions.collectionID || '',
environment: runOptions.environmentID || '',
iterations: _executionToIterationConverter(runSummary.run.executions, runOptions.iterationCount , runOptions.publishSkipResponse),
delay: runOptions.delayRequest || 0,
persist: false,
saveResponse: !runOptions.publishSkipResponse,
dataFile: runOptions.collectionUID && runOptions.environmentUID ? runOptions.iterationData : null,
workspace: runOptions.publish,
currentIteration: runOptions.iterationCount,
folder: null // This is always null as you cannot run a folder via newman
}
return JSON.stringify(run);
}

/**
* @public
* Starts the run upload process
*
* @param {*} runSummary
* @param {*} runOptions
*
* @returns { Promise }
*/
uploadRunToPostman = (runSummary, runOptions) => {
let runPayload;

runOptions.collectionID = util.extractResourceID(runOptions.collection);
runOptions.environmentID = util.extractResourceID(runOptions.environment);

try{
runPayload = _buildPostmanUploadPayload(runOptions, runSummary)
}catch(error){
throw new Error(`Unable to serialize the run - ${error}`);
}

print.lf('Uploading newman run to Postman');

const uploadOptions = {
url: UPLOAD_RUN_API_URL,
body: runPayload,
headers: {
'Content-Type': 'application/json',
'X-Api-Key': runOptions.postmanApiKey
}
},
retryOptions = {
maxRetries: runOptions.publishRetries,
totalTimeout: runOptions.publishTimeout,
retryDelayMultiplier: 2,
maxRetryDelay : 64,
addJitter: true
};

return _uploadWithRetry(_upload(uploadOptions), retryOptions);
}

module.exports = {
uploadRunToPostman,
POSTMAN_UPLOAD_ERROR_LINK,

// Exporting following functions for testing ONLY.
_buildPostmanUploadPayload,
_executionToIterationConverter,
_buildRequestObject,
_buildResponseObject,
_buildTestObject,
_uploadWithRetry,
_upload,

};
49 changes: 49 additions & 0 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,55 @@ util = {
*/
isFloat: function (value) {
return (value - parseFloat(value) + 1) >= 0;
},

/**
* Helper to get resource ID from URL
*
* If resourceUrl = https://api.getpostman.com/collections/<collection-UID>?apikey=<api_key>
* This method extracts <collection-UID>
*
* @param {String} resource - url for collection\environment
* @param resourceURL
* @returns {String}
*/
extractResourceID: function (resourceURL) {
if (_.isObject(resourceURL) || !resourceURL || resourceURL === '') {
Copy link
Member

Choose a reason for hiding this comment

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

This seems like a weird check. If we want the resourceURL to be a string, we should check it as such. Also, the case of it being empty string would not need to be handled explicitly at all since the regex would take care of it.

Suggested change
if (_.isObject(resourceURL) || !resourceURL || resourceURL === '') {
if (typeof resourceURL !== 'string') {

return '';
}

// eslint-disable-next-line max-len
const uidExtractionPattern = /https:\/\/api\.getpostman\.com\/(?:(?:collections)|(?:environments))\/([A-Za-z0-9-]+)\?\w*/g;

let matches,
results = [];

while ((matches = uidExtractionPattern.exec(resourceURL)) !== null) {
Copy link
Member

Choose a reason for hiding this comment

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

Why are we looping over all the matches? It should be a single match, no? You would have the match present in the first call to exec itself.

if (matches.index === uidExtractionPattern.lastIndex) {
uidExtractionPattern.lastIndex++;
}

matches.forEach((match) => { return results.push(match); });
}

return results.length === 0 ? '' : results[1];
},

partition: function (arr, n) {
const validPartitionArg = Number.isSafeInteger(n) && n > 0,
partitions = [];

if (!validPartitionArg) {
throw new Error('n must be positive integer');
}

for (let i = 0; i < arr.length; i += n) {
const partition = arr.slice(i, i + n);

partitions.push(partition);
}

return partitions;
}
};

Expand Down
Loading