added observability and session check in browserstack service#10037
added observability and session check in browserstack service#10037christian-bromann merged 12 commits intowebdriverio:mainfrom
Conversation
added observability and browserstack check
| capability['bstack:options'] = { wdioService: bstackServiceVersion } | ||
| } else if (shouldAddServiceVersion(this._config, this._options.testObservability)) { | ||
| capability['browserstack.wdioService'] = bstackServiceVersion | ||
| if (isBStackSession(this._config)) { |
There was a problem hiding this comment.
Instead of adding another nested if here, why not just have this at the beginning of the constructor and return early if the session is not a BS session.
There was a problem hiding this comment.
I created #10039 .. we could simplify the implementation by tackling that first, up to you.
There was a problem hiding this comment.
@christian-bromann the nested if condition is for checking whether we need to set wdioService version or not. After if condition there is code to set the buildIdentifier and buildName which we want to set even if it is not a browserstack session. So moving this to constructor won't help.
There was a problem hiding this comment.
After if condition there is code to set the
buildIdentifierandbuildNamewhich we want to set even if it is not a browserstack session.
Can you help me understand why this service wants to set these properties even if the session is not run on BS?
There was a problem hiding this comment.
We have a product called observability, which shows insights of tests. This is independent of the platform in which the tests are run. So tests can run browserstack, local or somewhere else, we want the collect tests data. So that's why we are setting buildIdentifier and buildName even if it is not a browserstack session.
There was a problem hiding this comment.
Gotcha, makes sense. Mind adding a comment for outside folks who are not familiar with this?
There was a problem hiding this comment.
So just to understand this correctly. Users can use the BS service even though they run their tests on a local chrome browser without having BS involved at all?
There was a problem hiding this comment.
Yes @christian-bromann, it's used for reporting like mochawesome
There seems to be a linting error. |
|
@christian-bromann Have fixed the linting issue |
|
@christian-bromann getting unrelated failure for windows and mac, possible to re-run the tests? Mac tests are passing when tested locally. |
|
These errors are unrelated and need to be fixed in |
@christian-bromann Could you please share ETA for releasing this PR and related v7 PR. |
next week latest |
|
@christian-bromann thanks for sharing the ETA. Actually, this is a bug for BrowserStack Test Observability users wherein they're not able to use the product if their tests are not running on BrowserStack infra. Hence, I just wanted to confirm the ETA both for this PR and the v7 PR as early next week so that we can share the same with the impacted users. |
Proposed changes
Previously we assumed that anyone with browserstack service installed wants to run their tests on browserstack only, now we want to allow people run tests locally along with browserstack service for observability. Made necessary changes for that.
Related V7 PR: #10038
Types of changes
Checklist
Further comments
Reviewers: @webdriverio/project-committers