Skip to content
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

Support for record flag #284

Merged
merged 15 commits into from
Apr 27, 2022
Merged

Conversation

roshan04
Copy link
Contributor

No description provided.

@nagpalkaran95 nagpalkaran95 changed the title Record flag changes Support for record flag Mar 23, 2022
@@ -241,6 +241,14 @@ const validate = (bsConfig, args) => {
logger.warn(Constants.validationMessages.SPEC_TIMEOUT_NOT_PASSED_ERROR);
}

if(!Utils.isUndefined(bsConfig.run_settings["record"])) {
Copy link
Member

Choose a reason for hiding this comment

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

if bsConfig.run_settings["record"] is false then I believe, it will go inside this code block and give warnings to users if the project Id or record key is missing which it ideally shouldn't

if(!Utils.isUndefined(bsConfig.run_settings["record"])) {
if(Utils.isUndefined(bsConfig.run_settings.projectId)) {
logger.warn(Constants.validationMessages.PROJECT_ID_MISSING);
} else if (Utils.isUndefined(bsConfig.run_settings["record-key"])) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's make this a separate if condition and not else if. The reason for this is to let users know on the first try only they are missing both projectId and record-key if they are missing.

exports.setProjectId = (bsConfig, args) => {
if(!this.isUndefined(args["projectId"])) {
return args["projectId"];
} else if(!this.isUndefined(bsConfig.run_settings["projectId"])) {
Copy link
Member

Choose a reason for hiding this comment

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

Just like env var CYPRESS_RECORD_KEY, we have an env var for project id i.e. CYPRESS_PROJECT_ID. More info is available here.

@@ -241,6 +241,15 @@ const validate = (bsConfig, args) => {
logger.warn(Constants.validationMessages.SPEC_TIMEOUT_NOT_PASSED_ERROR);
}

if(!Utils.isUndefined(bsConfig.run_settings["record"]) && String(bsConfig.run_settings["record"]) == 'true') {
if(Utils.isUndefined(bsConfig.run_settings.projectId)) {
Copy link
Member

Choose a reason for hiding this comment

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

let's add check for projectId or record-key being empty strings as well and show the warning.

pranavj1001
pranavj1001 previously approved these changes Mar 24, 2022
nagpalkaran95
nagpalkaran95 previously approved these changes Apr 18, 2022
Copy link
Collaborator

@nagpalkaran95 nagpalkaran95 left a comment

Choose a reason for hiding this comment

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

recent changes LGTM

nagpalkaran95
nagpalkaran95 previously approved these changes Apr 27, 2022
@francisf francisf merged commit 36e216a into browserstack:master Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants