add: dedicated method for parsing reporters

[elit-altum]

@shamasis @codenirvana

This is an alternate way of handling #2668 using variadic parameters. I think this is a cleaner solution and achieves the same results.

Please have a look!

[codecov[bot]]

Codecov Report

Merging #2673 (69533ca) into develop (ea8a8cd) will increase coverage by 0.06%. The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop    #2673      +/-   ##
===========================================
+ Coverage    90.75%   90.81%   +0.06%     
===========================================
  Files           21       21              
  Lines         1114     1122       +8     
  Branches       340      342       +2     
===========================================
+ Hits          1011     1019       +8     
  Misses         103      103              

Flag	Coverage Δ
cli	82.08% <66.66%> (-0.33%)	:arrow_down:
integration	41.62% <0.00%> (-0.30%)	:arrow_down:
library	59.62% <0.00%> (-0.43%)	:arrow_down:
unit	76.55% <100.00%> (+0.16%)	:arrow_up:

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

Impacted Files	Coverage Δ
bin/newman.js	89.79% <ø> (ø)
bin/util.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ea8a8cd...69533ca. Read the comment docs.

[adityaofficial10]

@elit-altum It has been already discussed that variadic method is not an option as that would be a breaking change Check this out: https://github.com/postmanlabs/newman/pull/2668

[elit-altum]

@elit-altum It has been already discussed that variadic method is not an option as that would be a breaking change Check this out: #2668

I know that @adityaofficial10, I authored that PR 😅

But this works similar to the previous command and gives the same functionality, which is what we were looking for.

[shamasis]

At a first glance @elit-altum this PR looks exactly like what and how we wanted it.

Get the final approval from your GSOC mentor and get this merged? It will be good.

[elit-altum]

At a first glance @elit-altum this PR looks exactly like what and how we wanted it.

Get the final approval from your GSOC mentor and get this merged? It will be good.

Thank you for the review @shamasis!

@codenirvana Could you please have a look at this so we can merge this?

[shamasis]

One thing though ... I haven't tested or checked whether any external usage has changed or not. Please check the tests and also ensure that existing usage doesn't require changes

[elit-altum]

@shamasis sure, I would once again test this and add new unit tests for the same!

[adityaofficial10]

@elit-altum Bro try to read the logs of Travis CI. That would help. From my view, I think there's an ESLint error somewhere related to maxlen. Also one of the unit tests is failing. Just check if the change you made to reporter option has been updated there. This should help.😄

[abhijit-hota]

Why do we need to split according to the commas if we're using variadic parameters?

P.S. I assume it's to not introduce any breaking changes?

[abhijit-hota]

@elit-altum Bro try to read the logs of Travis CI. That would help. From my view, I think there's an ESLint error somewhere related to maxlen. Also one of the unit tests is failing. Just check if the change you made to reporter option has been updated there. This should help.😄

I think that can be solved by introducing newlines like this:

.option('-r, --reporters [reporters...]', 
        'Specify the reporters to use for this run',
        util.cast.reporterParse, ['cli'])

[Sharath-Postman]

@elit-altum do create an issue (topic) on community.postman.com gsoc category under open technologies. One of the mentors from the newman team will review this

[raghavgarg1257]

@elit-altum Can you please fix the test to add the code coverage for the code that has been added for this PR? Code Ref: https://github.com/postmanlabs/newman/blob/develop/test/unit/cli.test.js#L208

[elit-altum]

@elit-altum Can you please fix the test to add the code coverage for the code that has been added for this PR? Code Ref: https://github.com/postmanlabs/newman/blob/develop/test/unit/cli.test.js#L208

Sure @raghavgarg1257 are you suggesting something like changing this line to something like: --reporters json, cli

and then add a line after this one to check for a cli reporter as well ?

[raghavgarg1257]

@elit-altum Yes, but maybe you will have to use other reporters for testing because cli has some conflicting config already present there.

[elit-altum]

@raghavgarg1257 Added the unit tests! I have included progress as the other reporter as there was already a test to ensure junit is not included.