Feature: Added a new default reporter to publish newman run to postman #2978

Harsh-Bansal · 2022-05-26T15:26:56Z

This default reporter can be used to publish newman run to postman.
This reporter doesn't have any associated options.

This reporter can be used to publish newman run to postman.

codecov · 2022-05-26T15:27:02Z

Codecov Report

Merging #2978 (18e8fe3) into develop (a5018ec) will increase coverage by 0.28%.
The diff coverage is 93.66%.

@@             Coverage Diff             @@
##           develop    #2978      +/-   ##
===========================================
+ Coverage    90.96%   91.25%   +0.28%     
===========================================
  Files           21       25       +4     
  Lines         1151     1292     +141     
  Branches       349      380      +31     
===========================================
+ Hits          1047     1179     +132     
- Misses         104      113       +9

Flag	Coverage Δ
cli	`76.16% <27.46%> (-6.03%)`	⬇️
integration	`39.31% <26.05%> (-1.69%)`	⬇️
library	`55.26% <28.87%> (-3.30%)`	⬇️
unit	`78.40% <93.66%> (+3.34%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
lib/run/index.js	`93.75% <ø> (ø)`
lib/reporters/postman/helpers/upload-run.js	`83.33% <83.33%> (ø)`
lib/reporters/postman/index.js	`93.54% <93.54%> (ø)`
lib/reporters/postman/helpers/run-utils.js	`95.00% <95.00%> (ø)`
lib/reporters/postman/helpers/constants.js	`100.00% <100.00%> (ø)`
lib/run/options.js	`95.45% <100.00%> (+0.03%)`	⬆️
lib/util.js	`96.29% <100.00%> (+1.05%)`	⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

Harsh-Bansal · 2022-05-26T15:28:25Z

@anshuljain03 will be pushing another commit for tests.
Core logic of the reporter is done, might update a few error msgs.

1. Moved upload-run to helpers folder 2. Code cleanup 3. Better error messages

codenirvana · 2022-06-01T09:38:11Z

lib/run/index.js

@@ -42,7 +42,8 @@ var _ = require('lodash'),
 json: require('../reporters/json'),
 junit: require('../reporters/junit'),
 progress: require('../reporters/progress'),
- emojitrain: require('../reporters/emojitrain')
+ emojitrain: require('../reporters/emojitrain'),
+ postman: require('../reporters/postman')


Are we sure the usage should be -r postman? What happened to --publish?

If yes, why it's named postman? 🤔

Using the -r postman follows the reporter convention which seems much better. Deprecating --publish would have been difficult (if needed).
Also, later when we add other configurable params (like timeout, retries), this approach would prevent bloating of newman cli options. They being specific to the reporter wont really belong as params of newman which doesnt use these (better separation of concerns on the interface)

Why postman: We went with postman as the data is being pushed to postman servers. (similar to other external reporters mentioned here (https://postmanlabs.atlassian.net/wiki/spaces/RUN/pages/3890544729))

We also considered cloudpublish/postmanpublish/postmancloud but postman seemed better.
Please let me know if you have any suggestions on this.

codenirvana · 2022-06-01T09:40:31Z

lib/reporters/postman/index.js

+
+ // We get the collection id in the collectionRunOptions. But that collection id has the user id stripped off
+ // Hence, we try to parse the CLI args to extract the whole collection id from it.
+ const processArgs = process.argv,


How will this work when Newman is used as a library?

This will be available only via CLI.
The issue is we need the collection UID to publish runs (wrt a collection) and we can get it from CLI args but when used as a library we don't have a way to get it. (The reporter gets passed the collection object fetched via postman api which doesnt have the UID)

Will figure out a way to use it as part of library also in the second iteration.
(Our current target use-case is CLI invocations as part of CI/CD)

@codenirvana have made the changes to make the reporter compatible with library also.
cc @anshuljain03 @shubhbhargav

codenirvana · 2022-06-01T09:42:15Z

lib/reporters/postman/index.js

+ collectionRunOptions.postmanApiKey || util.getAPIKeyFromCLIArguments(processArgs);
+
+ if (!collection) {
+ print.lf('Publishing run details to postman cloud is currently supported only for collections specified ' +


Are we following the Newman logging pattern here?

codenirvana · 2022-06-01T09:43:54Z

lib/reporters/postman/helpers/constants.js

+ /**
+ * The base URL for postman API
+ */
+ POSTMAN_API_BASE_URL: 'https://api.getpostman.com',


Why are we using the older getpostman.com domain name?

Switched to postman.com, thanks
Have also modified related regexs to support both getpostman.com and postman.com.

codenirvana · 2022-06-01T09:44:21Z

lib/reporters/postman/helpers/constants.js

+ /**
+ * Used as a fall back error message for the upload API call
+ */
+ RESPONSE_FALLBACK_ERROR_MESSAGE: 'Something went wrong while uploading newman run data to Postman',


No, "Something went" error messages, please.

codenirvana · 2022-06-01T09:45:25Z

lib/reporters/postman/helpers/constants.js

+ /**
+ * Regex pattern to extract the api key from the postman api collection url
+ */
+ API_KEY_FROM_URL_EXTRACTION_PATTERN:


Avoid this!

@codenirvana we are using this only as a fallback in case apikey is not present as environment variable POSTMAN_API_KEY or passed as cli arg --postman-api-key

Lemmi know if that's fine.

codenirvana · 2022-06-01T09:46:40Z

lib/reporters/postman/helpers/run-utils.js

+ time: response.responseTime,
+ size: response.responseSize,
+ headers: headers,
+ body: response.stream ? new TextDecoder('utf-8').decode(new Uint8Array(response.stream)) : null


response.text()

1. Changed fallback error message 2. Changed POSTMAN_API_BASE_URL to latest domain 3. Modified regex patterns to comply with both old and new domain 4. Modified _buildResponseObject to use response.text()

…t uids and apikey from postman reporter. This info will now be stored in options which is sent to the reporter. Moved utility functions to extract the above from postman reporter to newman util. Added test cases for the utility functions. Cleanup for existing tests based on above.

shamasis · 2022-06-17T09:31:16Z

lib/util.js

@@ -53,7 +53,19 @@ var fs = require('fs'),

 // Matches valid Postman UID, case insensitive.
 // Same used for validation on the Postman API side.
- UID_REGEX = /^[0-9A-Z]+-[0-9A-F]{8}-[0-9A-F]{4}-[0-9A-F]{4}-[0-9A-F]{4}-[0-9A-F]{12}$/i;


@codenirvana ... I think we should start moving away from having internal reporters using non public API. For example, the CLI reporter uses util too. That is what's prompting this. Instead, we should try opening these needed utils as reporter util exposed via newman itself.

Thoughts?

shamasis · 2022-06-17T09:32:12Z

lib/run/options.js

@@ -380,6 +380,13 @@ module.exports = function (options, callback) {
 // allow insecure file read by default
 options.insecureFileRead = Boolean(_.get(options, 'insecureFileRead', true));

+ // store collection, environment uid and postman api key in options if specified using postman public api link
+ options.cachedArgs = {


Premature execution. Alternate source available for this data from SDK instances inside. cachedArgs elevated/leaked in reporter facing public API

shamasis · 2022-06-17T09:34:35Z

lib/util.js

+ UID_REGEX = /^[0-9A-Z]+-[0-9A-F]{8}-[0-9A-F]{4}-[0-9A-F]{4}-[0-9A-F]{4}-[0-9A-F]{12}$/i,
+
+ // Regex pattern to extract the collection id from the postman api collection url
+ COLLECTION_UID_FROM_URL_EXTRACTION_PATTERN =


ReDos vulnerable RegEx

shamasis · 2022-06-17T09:35:46Z

lib/util.js

+ /https?:\/\/api\.(?:get)?postman.*\.com\/(?:collections)\/([A-Za-z0-9-]+)/,
+
+ // Regex pattern to extract the environment id from the postman api environment url
+ ENVIRONMENT_UID_FROM_URL_EXTRACTION_PATTERN =


ReDOS here probably too

shamasis · 2022-06-17T09:36:34Z

lib/util.js

+
+ // Regex pattern to extract the api key from the postman api collection url
+ API_KEY_FROM_URL_EXTRACTION_PATTERN =
+ /https?:\/\/api.(?:get)?postman.com\/[a-z]+s\/[a-z0-9-]+\?apikey=([a-z0-9A-Z-]+)/;


Seems not REDOS vulnerable 👍

shamasis · 2022-06-17T09:37:24Z

lib/util.js

@@ -284,6 +296,76 @@ util = {
 */
 isFloat: function (value) {
 return (value - parseFloat(value) + 1) >= 0;
+ },


Not relevant to global util.

Added a new default reporter 'Postman'

7ef05aa

This reporter can be used to publish newman run to postman.

Harsh-Bansal requested a review from anshuljain03 May 26, 2022 15:26

Harsh-Bansal self-assigned this May 26, 2022

Minor changes in Postman reporter:

ca91cdf

1. Moved upload-run to helpers folder 2. Code cleanup 3. Better error messages

Harsh-Bansal requested a review from codenirvana May 27, 2022 05:28

harsh-bansal97 added 2 commits May 30, 2022 17:15

Added unit tests for postman-reporter related functions

5c32b30

Added unit tests for Postman reporter

451590a

codenirvana requested changes Jun 1, 2022

View reviewed changes

harsh-bansal97 added 2 commits June 3, 2022 08:38

Addressed review:

163404b

1. Changed fallback error message 2. Changed POSTMAN_API_BASE_URL to latest domain 3. Modified regex patterns to comply with both old and new domain 4. Modified _buildResponseObject to use response.text()

shamasis requested changes Jun 17, 2022

View reviewed changes

DannyDainton added the product/newman label Jul 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Added a new default reporter to publish newman run to postman #2978

Feature: Added a new default reporter to publish newman run to postman #2978

Harsh-Bansal commented May 26, 2022

codecov bot commented May 26, 2022 •

edited

Harsh-Bansal commented May 26, 2022

codenirvana Jun 1, 2022

Harsh-Bansal Jun 2, 2022

codenirvana Jun 1, 2022

Harsh-Bansal Jun 2, 2022

Harsh-Bansal Jun 6, 2022

codenirvana Jun 1, 2022

codenirvana Jun 1, 2022

Harsh-Bansal Jun 3, 2022

codenirvana Jun 1, 2022

codenirvana Jun 1, 2022

Harsh-Bansal Jun 2, 2022

codenirvana Jun 1, 2022

shamasis Jun 17, 2022

shamasis Jun 17, 2022

shamasis Jun 17, 2022

shamasis Jun 17, 2022

shamasis Jun 17, 2022

shamasis Jun 17, 2022

Feature: Added a new default reporter to publish newman run to postman #2978

Are you sure you want to change the base?

Feature: Added a new default reporter to publish newman run to postman #2978

Conversation

Harsh-Bansal commented May 26, 2022

codecov bot commented May 26, 2022 • edited

Codecov Report

Harsh-Bansal commented May 26, 2022

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

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

codecov bot commented May 26, 2022 •

edited