-
Notifications
You must be signed in to change notification settings - Fork 3
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
Reporter debug mode #12
Conversation
Sure, let me take a look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tomaszsluszniak - Have commented some things, rest everything looks okay. I can make a release once changes are done.
src/influxdb-reporter.js
Outdated
@@ -116,10 +119,15 @@ class InfluxDBReporter { | |||
if(error) { | |||
this.context.currentItem.data.test_status = 'FAIL'; | |||
|
|||
const failMessage = `${error.test} | ${error.name}: ${error.message}`; | |||
var failMessage = `${error.test} | ${error.name}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we let
instead of var
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed
this.service.sendData(binaryData); | ||
} | ||
|
||
done() { | ||
this.service.disconnect(); | ||
console.log(`[+] Finished collection: ${this.options.collection.name} (${this.context.id})`); | ||
|
||
// console.log('this.context', this.context); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for removing commented stuff
src/influxdb-reporter.js
Outdated
@@ -20,12 +20,15 @@ class InfluxDBReporter { | |||
failed: [], | |||
skipped: [] | |||
}, | |||
list: [] | |||
list: [], | |||
debug: this.reporterOptions.debug |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe move this to start() method, as other --reporter-{...} declaration are specified there..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can have statement like this:
this.context.debug = this.reporterOptions.debug || this.reporterOptions.debug || false;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed, but I would leave it like that because I'm already using debug
in the constructor's if-statement.
https://github.com/tomaszsluszniak/newman-reporter-influxdb/blob/4ef4537f594c2677fba813bf8a993f3e35e5b738/src/influxdb-reporter.js#L30
@tomaszsluszniak - version 2.0.2 released with debug changes! Please verify. |
Hi,
could you consider merging this? We've added a debug mode which is really useful in our scenario. Maybe it could be useful also for others.
Just add
debug
option inreporterOptions
to make the output a bit more verbose.