Fix browser version in spec reporter

[unickq]

Before:

[chrome  windows #0-0] Spec: C:\Git\integration-tests\bolt\integration-tests\checkout\specs\magento2\front\cart\xxx.ts
[chrome  windows #0-0] Running: chrome on windows
[chrome  windows #0-0] Session ID: 5afa1aa20220c2e4a26ca039c9149f33

After:

[chrome 81.0.4044.138 windows #0-0] Spec: C:\Git\integration-tests\bolt\integration-tests\checkout\specs\magento2\front\cart\xxx.ts
[chrome 81.0.4044.138 windows #0-0] Running: chrome (v81.0.4044.138) on windows
[chrome 81.0.4044.138 windows #0-0] Session ID: b4b86051d1fca73c0c2d3dca6f01f80c

Proposed changes

Types of changes

• Bugfix (non-breaking change which fixes an issue)

Checklist

• I have read the CONTRIBUTING doc
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)
• I have added proper type definitions for new commands (if appropriate)

Further comments

Reviewers: @webdriverio/project-committers

[jsf-clabot]

All committers have signed the CLA.

[christian-bromann]

I would recommend to change the line to:

const version = capss.browserVersion || caps.version || '(unknown)'

[erwinheitzman]

I see some confusion, let's clear that up right away to prevent this in the future

[erwinheitzman]

Left a question, if my suspicion is correct then we can simplify it a little further by always taking the OS except when a device is used, in which case it will take the platform or platformName :)

[erwinheitzman]

Great and easy fix btw, thanks for the contribution 👍

[christian-bromann]

let's keep the definiton of platform and version at one place

[erwinheitzman]

fair enough, the comments you added add more value than this!

[christian-bromann]

I missed something in my suggestion. Also can we add a test to check support for BS invalid caps: browser_version and os

[unickq]

@christian-bromann updated your platform to:

const platform = caps.os ? (caps.os + ' ' + caps.osVersion) : caps.platformName || caps.platform

So no undefined in platform anymore

[safari 13605.3.8 macOS #3-0] Spec: C:\Git\integration-tests\bolt\integration-tests\checkout\specs\magento2\front\cart\xxx.ts
[safari 13605.3.8 macOS #3-0] Running: safari (v13605.3.8) on macOS
[safari 13605.3.8 macOS #3-0] Session ID: a2760e25cf7d172c5b77c45e4f0d62c4b2dab5fe
------------------------------------------------------------------
[firefox 76.0 windows #2-0] Spec: C:\Git\integration-tests\bolt\integration-tests\checkout\specs\magento2\front\cart\xxx.ts
[firefox 76.0 windows #2-0] Running: firefox (v76.0) on windows
[firefox 76.0 windows #2-0] Session ID: ec7089d90c1068233cad9e10252cef95c823b05e
------------------------------------------------------------------
[iPhone XR iOS 12.0 #0-0] Spec: C:\Git\integration-tests\bolt\integration-tests\checkout\specs\magento2\front\cart\xxx.ts
[iPhone XR iOS 12.0 #0-0] Running: iPhone XR on iOS 12.0 executing safari
[iPhone XR iOS 12.0 #0-0] Session ID: bda23c72821576a7670c43c6095efd710f6000cf
------------------------------------------------------------------
[8ATX0Z2CV Android 10 #1-0] Spec: C:\Git\integration-tests\bolt\integration-tests\checkout\specs\magento2\front\cart\xxx.ts
[8ATX0Z2CV Android 10 #1-0] Running: 8ATX0Z2CV on Android 10 executing chrome
[8ATX0Z2CV Android 10 #1-0] Session ID: f0ea886d9d3b4b3050fe71e9dc914b0dda5be0ab

[christian-bromann]

@unickq would you mind to add a test where you use browser_version and os in your capabilities and snapshot the spec reporter output?

[erwinheitzman]

last remark for me

[erwinheitzman]

@unickq would you mind to add a test where you use browser_version and os in your capabilities and snapshot the spec reporter output?

@christian-bromann this test already exists here I think (all the way at the bottom)?
https://github.com/webdriverio/webdriverio/blob/master/packages/wdio-spec-reporter/tests/index.test.js

EDIT: those tests are actually failing atm since the version seems to be undefined

[erwinheitzman]

tests are failing

[erwinheitzman]

And like Christian said, for any changed functionality we should add some tests

[unickq]

And like Christian said, for any changed functionality we should add some tests

Sure I'll try to do this when I get my mac.
Like I said can't make build/lint/test on Windows somehow 😞

[erwinheitzman]

@unickq that's strange, it worked fine last time I tried on my Windows machine using vscode. I have changed the integrated terminal to be git bash but that's about it.

then run:

npm i
npm run setup-full
npx jest ./packages/wdio-spec-reporter/tests

NOTE: make sure you have NodeJS 10 or 12 I'd say

[unickq]

Thanks for help @erwinheitzman

VS integrated powershell didn't work. But external did.
I updated tests with new (unknown) state. And like you said before - browserstack tests are already there.

Can you please confirm I'm doing right things with the tests?

[christian-bromann]

I updated tests with new (unknown) state.

Can you please update the tests to use proper capabilities to actually test the change?

[unickq]

@christian-bromann

I checked coverage report:

And it looks like we can never reach that branch, because

const platform = caps.platformName || caps.platform || (caps.os ? caps.os + ' ' + caps.os_version : '(unknown)')

returns (unknown) if nothing else is available.

Doesn't it mean lines 417-421 needs to be updated to

        if (!verbose) {
            return (browser + ' ' + (version || '') + ' ' + (platform)).trim()
        }

        return browser + (version ? ` (v${version})` : '') + (` on ${platform}`)

and no additional coverage tests are needed?

[christian-bromann]

@unickq please work with various capabilities to test different report outputs. Currently we only work with one capability but we could use multiple to ensure that these changes are included in the tests.

[christian-bromann]

can we not have unknown in the test snapshot and instead use an actual capability?

[christian-bromann]

LGTM 👍

Thank you for your patience with us. I wanted to make sure we get this right.