Skip to content
This repository has been archived by the owner on Mar 13, 2020. It is now read-only.

use single -s arg with comma separated list of sauce browsers #158

Closed
wants to merge 1 commit into from

Conversation

frankiefu
Copy link
Member

@frankiefu frankiefu commented Jun 15, 2018

From @usergenic 's comment: #155 (review)
See if this fixes the test issue.

@frankiefu frankiefu requested a review from keanulee as a code owner June 15, 2018 17:30
Copy link
Contributor

@notwaldorf notwaldorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if the tests are green :D

@frankiefu
Copy link
Member Author

🙏

@frankiefu
Copy link
Member Author

frankiefu commented Jun 15, 2018

@usergenic Tried to use comma separated list but it didn't work. Can you help us to figure out the right command to use?
Also noticed there was a polymer-cli release 3 days ago so maybe something was changed there.

@notwaldorf
Copy link
Contributor

notwaldorf commented Jun 15, 2018

Now it's complaining these values don't match:

windows 10 microsoftedge 17,macos 10.13/safari@11 Tests failed: Unsupported
OS/browser/version/device combo: OS: 'Windows 10', Browser: 'microsoftedge', Version: '17,macos 
10.13/safari@11.', Device: 'unspecified'

@usergenic
Copy link
Contributor

I was wrong about comma separated values.

I haven't 100% confirmed the history but given the usage, the argument in cli's test command code should have been specified as 'multiple: true' but it wasn't-- I think a version update in recent package lock refresh must have brought in change to command-line-args library which now enforces single values unless specified as multiple.

I tested the change locally and am going to try to get fixed in next release Polymer/tools#515

@usergenic
Copy link
Contributor

Tested fix by making -s "multiple: true"...

λ npx polymer test --module-resolution=node --npm -s 'windows 10/microsoftedge@17' -s 'macos 10.13/safari@11'
Creating Sauce Connect tunnel
Sauce Connect log: /tmp/wct118516-192250-1xi2lfy.9444f/sc.log
Sauce tunnel active: fdfcd716-a268-4c59-957e-bf4d92c66e2c
[BABEL] Note: The code generator has deoptimised the styling of undefined as it exceeds the max of 500KB.
macos 10.13 safari 11    Beginning tests via http://localhost:8081/components/pwa-starter-kit/generated-index.html?cli_browser_id=1
[BABEL] Note: The code generator has deoptimised the styling of undefined as it exceeds the max of 500KB.
windows 10 microsoftedge 17 Beginning tests via http://localhost:8081/components/pwa-starter-kit/generated-index.html?cli_browser_id=0
macos 10.13 safari 11    Tests passed
windows 10 microsoftedge 17 Tests passed
Test run ended with great success

windows 10 microsoftedge 17 (16/0/0)    macos 10.13 safari 11 (16/0/0)

@frankiefu
Copy link
Member Author

@usergenic Awesome! Thanks for looking into it.

@frankiefu frankiefu closed this Jun 18, 2018
@notwaldorf notwaldorf deleted the wct-sauce branch June 18, 2018 17:22
@usergenic
Copy link
Contributor

@frankiefu @notwaldorf wct 6.7.0 and polymer-cli 1.7.4 have both just been published which should solve this issue, i.e. multiple -s arguments accepted again.

@frankiefu
Copy link
Member Author

@usergenic Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants