O11y Artifact changes #3
Conversation
| ``` | ||
|
|
||
| You can explore all the features of Test Observability in [this sandbox](https://observability-demo.browserstack.com/) or read more about it [here](https://www.browserstack.com/docs/test-observability/overview/what-is-test-observability). | ||
| You can explore all the features of Test Reporting and Analytics in [this sandbox](https://automation.browserstack.com/) or read more about it [here](https://www.browserstack.com/docs/test-observability/overview/what-is-test-observability). |
There was a problem hiding this comment.
| if (this._options.testReporting === undefined && this._options.testObservability !== undefined) { | ||
| this._options.testReporting = this._options.testObservability | ||
| } else if (this._options.testReporting !== undefined && this._options.testObservability === undefined) { | ||
| this._options.testObservability = this._options.testReporting |
There was a problem hiding this comment.
Any reason for managing 2 options here?
There was a problem hiding this comment.
We have to support both, testObservability and testReporting flag, and options come with the flag name they are using.
I am assuming customers won't know about testReportingOptions if they are using testObservability flag so they will use testObservabilityOptions and we have to add backward compatibility for the old flag.
| * @deprecated Use testReporting instead | ||
| */ | ||
| testObservability?: boolean; | ||
| testObservability?: boolean | { enabled: boolean }; |
There was a problem hiding this comment.
Why is | { enabled: boolean } added here?
| return process.env.BROWSERSTACK_TEST_REPORTING === 'true' | ||
| } | ||
| if (process.env.BROWSERSTACK_OBSERVABILITY !== undefined) { | ||
| return process.env.BROWSERSTACK_OBSERVABILITY === 'true' |
There was a problem hiding this comment.
Do we really need to manitain 2 different vars?
There was a problem hiding this comment.
yes requirement of story is both should work
| if (typeof options.testObservability === 'object' && options.testObservability !== null) { | ||
| return options.testObservability.enabled || false | ||
| } | ||
| return Boolean(options.testObservability) |
There was a problem hiding this comment.
Any reason for not maintaining it in single var ultimately
There was a problem hiding this comment.
we get vars from config itself thats how we consume from user, hence used two vars
Proposed changes
Types of changes
Checklist
Backport Request
//: # (The current
mainbranch is the development branch for WebdriverIO v9. If your change should be released to the current major version of WebdriverIO (v8), please raise another PR with the same changes against thev8branch.)v9and doesn't need to be back-ported#XXXXXFurther comments
Reviewers: @webdriverio/project-committers