-
Notifications
You must be signed in to change notification settings - Fork 39
specs and env options #46
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
Conversation
bin/helpers/utils.js
Outdated
let bsConfigSpecs = bsConfig.run_settings.specs; | ||
|
||
if (!this.isUndefined(args.specs)) { | ||
bsConfig.run_settings.specs = args.specs.split(/\s{0,},\s+/).join(','); |
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.
Check for array as well just like bsConfig.run_settings.specs
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.
cli arg cannot be an array, we defined it as a string in https://github.com/browserstack/browserstack-cypress-cli/pull/46/files/961eb70b7347802f7cad51a6192c92b6ec21df39#diff-e2a46d3c5c3e72a2ddd20518f32e5ac9R171
bin/helpers/utils.js
Outdated
// env option must be set only from command line args as a string | ||
exports.setTestEnvs = (bsConfig, args) => { | ||
if (!this.isUndefined(args.env)) { | ||
bsConfig.run_settings.env = args.env.split(/\s{0,},\s+/).join(','); |
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.
.split(/\s{0,},\s+/).join(',')
move this to a func? being used 3 time in the last last lines of code
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.
Done
describe: Constants.cliMessages.RUN.SPECS_DESCRIPTION, | ||
type: "string", | ||
default: undefined |
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.
confirm the cli messages and default values once
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.
Will update the message values.
@@ -249,6 +249,136 @@ describe("capabilityHelper.js", () => { | |||
chai.assert.fail("Promise error"); | |||
}); | |||
}); | |||
|
|||
context("specs and env from run_setting", () => { | |||
it("sets specs list is present", () => { |
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.
no test where specs list is an Array or passed args is of Array type
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.
As call to util.setUserSpecs
and util.setTestEnvs
are made in runs.js and then final state(String) is passed into capabilityHelper. Hence, capabilityHelper always receives string.
specs for array conditions are covered in the util.js itself: https://github.com/browserstack/browserstack-cypress-cli/pull/46/files/961eb70b7347802f7cad51a6192c92b6ec21df39#diff-175b2b2ef3763bf0f62fe44137aa5d34R350
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.
lgtm
This PR is good to go. Sorry for the delay in response. |
This PR adds: