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

Handle spec level timeouts #248

Merged
merged 11 commits into from
Mar 14, 2022
3 changes: 3 additions & 0 deletions bin/commands/runs.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ module.exports = function run(args, rawArgs) {
// set cypress geo location
utils.setGeolocation(bsConfig, args);

// set spec timeout
utils.setSpecTimeout(bsConfig, args);

// accept the specs list from command line if provided
utils.setUserSpecs(bsConfig, args);

Expand Down
14 changes: 14 additions & 0 deletions bin/helpers/capabilityHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,20 @@ const validate = (bsConfig, args) => {
addCypressZipStartLocation(bsConfig.run_settings);
}

if(!Utils.isUndefined(bsConfig.run_settings.spec_timeout)) {
if(Utils.isPositiveInteger(bsConfig.run_settings.spec_timeout.toString().trim())) {
if(Number(bsConfig.run_settings.spec_timeout) > Constants.SPEC_TIMEOUT_LIMIT) {
reject(Constants.validationMessages.SPEC_TIMEOUT_LIMIT_ERROR)
} else {
logger.info(Constants.userMessages.SPEC_LIMIT_SUCCESS_MESSAGE.replace("<x>", bsConfig.run_settings.spec_timeout));
}
} else {
logger.warn(Constants.userMessages.SPEC_TIMEOUT_LIMIT_WARNING)
}
} else {
logger.warn(Constants.validationMessages.SPEC_TIMEOUT_NOT_PASSED_ERROR);
}

resolve(cypressJson);
});
}
Expand Down
16 changes: 12 additions & 4 deletions bin/helpers/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,9 @@ const userMessages = {
DOWNLOAD_BUILD_ARTIFACTS_SUCCESS: "Your build artifact(s) have been successfully downloaded in '<user-path>/build_artifacts/<build-id>' directory",
LATEST_SYNTAX_TO_ACTUAL_VERSION_MESSAGE: "Your build will run using Cypress <actualVersion> as you had specified <latestSyntaxVersion>.<frameworkUpgradeMessage> Read more about supported versions here: http://browserstack.com/docs/automate/cypress/supported-versions",
PROCESS_KILL_MESSAGE: "Stopping the CLI and the execution of the build on BrowserStack",
BUILD_FAILED_ERROR: "The above stacktrace has been thrown by Cypress when we tried to run your build. If your test suite requires npm dependencies then please specify them on browserstack.json. Read more at " + chalk.blueBright("https://www.browserstack.com/docs/automate/cypress/npm-packages") + ". Also, we recommend you to try running the build locally using ‘cypress run’ and if it works fine then please reach out to support at " + chalk.blueBright("https://www.browserstack.com/contact#technical-support")
BUILD_FAILED_ERROR: "The above stacktrace has been thrown by Cypress when we tried to run your build. If your test suite requires npm dependencies then please specify them on browserstack.json. Read more at " + chalk.blueBright("https://www.browserstack.com/docs/automate/cypress/npm-packages") + ". Also, we recommend you to try running the build locally using ‘cypress run’ and if it works fine then please reach out to support at " + chalk.blueBright("https://www.browserstack.com/contact#technical-support"),
SPEC_TIMEOUT_LIMIT_WARNING: "Value for the 'spec_timeout' key not in the 1-120 range. Going ahead with 30 mins as the default spec timeout. Read more about how to specify the option in https://browserstack.com/docs/automate/cypress/spec-timeout",
SPEC_LIMIT_SUCCESS_MESSAGE: "Spec timeout specified as <x> minutes. If any of your specs exceed the specified time limit, it would be forcibly killed by BrowserStack"
};

const validationMessages = {
Expand Down Expand Up @@ -97,7 +99,9 @@ const validationMessages = {
NOT_ALLOWED_GEO_LOCATION_AND_LOCAL_MODE: "IP Geolocation feature is not available in conjunction with BrowserStack Local.",
HOME_DIRECTORY_NOT_FOUND: "Specified home directory could not be found. Please make sure the path of the home directory is correct.",
HOME_DIRECTORY_NOT_A_DIRECTORY: "Specified home directory is not a directory. The home directory can only be a directory and not a file.",
CYPRESS_CONFIG_FILE_NOT_PART_OF_HOME_DIRECTORY: "Could not find cypress.json within the specified home directory. Please make sure cypress.json resides within the home directory."
CYPRESS_CONFIG_FILE_NOT_PART_OF_HOME_DIRECTORY: "Could not find cypress.json within the specified home directory. Please make sure cypress.json resides within the home directory.",
SPEC_TIMEOUT_LIMIT_ERROR: "The maximum allowed value of 'spec_timeout' is 120. Read more on https://browserstack.com/docs/automate/cypress/spec-timeout ",
SPEC_TIMEOUT_NOT_PASSED_ERROR: "'spec_timeout' key not specified. Going ahead with 30 mins as the default spec timeout. Read more about how to specify the option in https://browserstack.com/docs/automate/cypress/spec-timeout "
};

const cliMessages = {
Expand Down Expand Up @@ -140,7 +144,8 @@ const cliMessages = {
CONFIG_DESCRIPTION: "Set configuration values. Separate multiple values with a comma. The values set here override any values set in your configuration file.",
REPORTER: "Specify the custom reporter to use",
REPORTER_OPTIONS: "Specify reporter options for custom reporter",
CYPRESS_GEO_LOCATION: "Enterprise feature to simulate website and mobile behavior from different locations."
CYPRESS_GEO_LOCATION: "Enterprise feature to simulate website and mobile behavior from different locations.",
SPEC_TIMEOUT: "Specify a value for a hard timeout for each spec execution in the 1-120 mins range. Read https://browserstack.com/docs/automate/cypress/spec-timeout for more details."
},
COMMON: {
DISABLE_USAGE_REPORTING: "Disable usage reporting",
Expand Down Expand Up @@ -234,6 +239,8 @@ const REDACTED = "[REDACTED]";

const REDACTED_AUTH =`auth: { "username": ${REDACTED}, "access_key": ${REDACTED} }`;

const SPEC_TIMEOUT_LIMIT = 120 // IN MINS

module.exports = Object.freeze({
syncCLI,
userMessages,
Expand All @@ -254,5 +261,6 @@ module.exports = Object.freeze({
ERROR_EXIT_CODE,
AUTH_REGEX,
REDACTED_AUTH,
BUILD_FAILED_EXIT_CODE
BUILD_FAILED_EXIT_CODE,
SPEC_TIMEOUT_LIMIT
});
33 changes: 33 additions & 0 deletions bin/helpers/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,23 @@ exports.setGeolocation = (bsConfig, args) => {
}
}

exports.isSpecTimeoutArgPassed = () => {
return this.searchForOption('--spec-timeout') || this.searchForOption('-t');
}
exports.setSpecTimeout = (bsConfig, args) => {
let specTimeout = null;
if(this.isSpecTimeoutArgPassed()) {
if(!this.isUndefined(args.specTimeout)) {
specTimeout = args.specTimeout;
} else {
specTimeout = 'undefined'
}
} else if (!this.isUndefined(bsConfig.run_settings.spec_timeout)) {
specTimeout = bsConfig.run_settings.spec_timeout;
}
bsConfig.run_settings.spec_timeout = specTimeout;
}

// specs can be passed from bstack configuration file
// specs can be passed via command line args as a string
// command line args takes precedence over config
Expand Down Expand Up @@ -390,10 +407,26 @@ exports.fixCommaSeparatedString = (string) => {

exports.isUndefined = value => (value === undefined || value === null);

exports.isPositiveInteger = (str) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

add tests for these too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

if (typeof str !== 'string') {
return false;
}

const num = Number(str);

if (this.isInteger(num) && num > 0) {
return true;
}

return false;
}

exports.isTrueString = value => (!this.isUndefined(value) && value.toString().toLowerCase() === 'true');

exports.isFloat = (value) => Number(value) && Number(value) % 1 !== 0;

exports.isInteger = (value) => Number.isInteger(value);
Copy link
Collaborator

Choose a reason for hiding this comment

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

add tests for these too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


exports.nonEmptyArray = (value) => {
if(!this.isUndefined(value) && value && value.length) {
return true;
Expand Down
6 changes: 6 additions & 0 deletions bin/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,12 @@ var argv = yargs
type: "string",
default: undefined
},
't': {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to add help section for this arg. Also specify mins when giving range in messgae

Copy link
Contributor Author

Choose a reason for hiding this comment

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

by help section do you mean the description have added it under SPEC_TIMEOUT

alias: ['specTimeout'],
default: undefined,
describe: Constants.cliMessages.RUN.SPEC_TIMEOUT,
type: "string"
},
'disable-npm-warning': {
default: false,
description: Constants.cliMessages.COMMON.NO_NPM_WARNING,
Expand Down
16 changes: 15 additions & 1 deletion test/unit/bin/commands/runs.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ describe("runs", () => {
setConfigStub = sandbox.stub();
setCLIModeStub = sandbox.stub();
setGeolocationStub = sandbox.stub();
setSpecTimeoutStub = sandbox.stub().returns(undefined);
});

afterEach(() => {
Expand Down Expand Up @@ -156,7 +157,8 @@ describe("runs", () => {
setBrowsers: setBrowsersStub,
setConfig: setConfigStub,
setCLIMode: setCLIModeStub,
setGeolocation: setGeolocationStub
setGeolocation: setGeolocationStub,
setSpecTimeout: setSpecTimeoutStub
},
'../helpers/capabilityHelper': {
validate: capabilityValidatorStub
Expand Down Expand Up @@ -197,6 +199,7 @@ describe("runs", () => {
sinon.assert.calledOnce(setLocalIdentifierStub);
sinon.assert.calledOnce(setUsageReportingFlagStub);
sinon.assert.calledOnce(setGeolocationStub);
sinon.assert.calledOnce(setSpecTimeoutStub);
sinon.assert.calledOnceWithExactly(
sendUsageReportStub,
bsConfig,
Expand Down Expand Up @@ -254,6 +257,7 @@ describe("runs", () => {
setCLIModeStub = sandbox.stub();
setGeolocationStub = sandbox.stub();
getVideoConfigStub = sandbox.stub();
setSpecTimeoutStub = sandbox.stub().returns(undefined);
});

afterEach(() => {
Expand Down Expand Up @@ -298,6 +302,7 @@ describe("runs", () => {
setCLIMode: setCLIModeStub,
setGeolocation: setGeolocationStub,
getVideoConfig: getVideoConfigStub,
setSpecTimeout: setSpecTimeoutStub
},
'../helpers/capabilityHelper': {
validate: capabilityValidatorStub,
Expand Down Expand Up @@ -357,6 +362,7 @@ describe("runs", () => {
sinon.assert.calledOnce(setDefaultsStub);
sinon.assert.calledOnce(setSystemEnvsStub);
sinon.assert.calledOnce(setGeolocationStub);
sinon.assert.calledOnce(setSpecTimeoutStub);
sinon.assert.calledOnceWithExactly(
sendUsageReportStub,
bsConfig,
Expand Down Expand Up @@ -416,6 +422,7 @@ describe("runs", () => {
fetchZipSizeStub = sandbox.stub();
setGeolocationStub = sandbox.stub();
getVideoConfigStub = sandbox.stub();
setSpecTimeoutStub = sandbox.stub().returns(undefined);
});

afterEach(() => {
Expand Down Expand Up @@ -461,6 +468,7 @@ describe("runs", () => {
fetchZipSize: fetchZipSizeStub,
setGeolocation: setGeolocationStub,
getVideoConfig: getVideoConfigStub,
setSpecTimeout: setSpecTimeoutStub
},
'../helpers/capabilityHelper': {
validate: capabilityValidatorStub,
Expand Down Expand Up @@ -522,6 +530,7 @@ describe("runs", () => {
sinon.assert.calledOnce(setDefaultsStub);
sinon.assert.calledOnce(setSystemEnvsStub);
sinon.assert.calledOnce(setGeolocationStub);
sinon.assert.calledOnce(setSpecTimeoutStub);
sinon.assert.calledOnceWithExactly(
sendUsageReportStub,
bsConfig,
Expand Down Expand Up @@ -586,6 +595,7 @@ describe("runs", () => {
fetchZipSizeStub = sandbox.stub();
setGeolocationStub = sandbox.stub();
getVideoConfigStub = sandbox.stub();
setSpecTimeoutStub = sandbox.stub().returns(undefined);
});

afterEach(() => {
Expand Down Expand Up @@ -632,6 +642,7 @@ describe("runs", () => {
fetchZipSize: fetchZipSizeStub,
setGeolocation: setGeolocationStub,
getVideoConfig: getVideoConfigStub,
setSpecTimeout: setSpecTimeoutStub
},
'../helpers/capabilityHelper': {
validate: capabilityValidatorStub,
Expand Down Expand Up @@ -704,6 +715,7 @@ describe("runs", () => {
sinon.assert.calledOnce(setDefaultsStub);
sinon.assert.calledOnce(setSystemEnvsStub);
sinon.assert.calledOnce(setGeolocationStub);
sinon.assert.calledOnce(setSpecTimeoutStub);

sinon.assert.calledOnceWithExactly(
sendUsageReportStub,
Expand Down Expand Up @@ -782,6 +794,7 @@ describe("runs", () => {
fetchZipSizeStub = sandbox.stub();
setGeolocationStub = sandbox.stub();
getVideoConfigStub = sandbox.stub();
setSpecTimeoutStub = sandbox.stub().returns(undefined);
});

afterEach(() => {
Expand Down Expand Up @@ -836,6 +849,7 @@ describe("runs", () => {
fetchZipSize: fetchZipSizeStub,
setGeolocation: setGeolocationStub,
getVideoConfig: getVideoConfigStub,
setSpecTimeout: setSpecTimeoutStub
},
'../helpers/capabilityHelper': {
validate: capabilityValidatorStub,
Expand Down
117 changes: 117 additions & 0 deletions test/unit/bin/helpers/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -2920,6 +2920,123 @@ describe('utils', () => {
});
});

describe('isSpecTimeoutArgPassed', () => {
let searchForOptionStub;
beforeEach(() => {
searchForOptionStub = sinon.stub(utils, 'searchForOption').withArgs('--spec-timeout');
})
afterEach(() => {
sinon.restore();
})
it('returns true if --spec-timeout flag is passed', () => {
searchForOptionStub.withArgs('--spec-timeout').returns(true);
expect(utils.isSpecTimeoutArgPassed()).to.eq(true);
});

it('returns true if -t flag is passed', () => {
searchForOptionStub.withArgs('--spec-timeout').returns(true);
searchForOptionStub.withArgs('-t').returns(true);
// stub2.returns(true);
expect(utils.isSpecTimeoutArgPassed()).to.eq(true);
});

it('returns false if no flag is passed', () => {
searchForOptionStub.withArgs('--spec-timeout').returns(false);
searchForOptionStub.withArgs('-t').returns(false);
expect(utils.isSpecTimeoutArgPassed()).to.eq(false);
});
});

describe("setSpecTimeout", () => {
let isSpecTimeoutArgPassedStub;
beforeEach(() => {
isSpecTimeoutArgPassedStub = sinon.stub(utils, 'isSpecTimeoutArgPassed');
});

afterEach(() => {
isSpecTimeoutArgPassedStub.restore();
});
it('sets spec_timeout defined value passed in args', () => {
let bsConfig = {
run_settings: {
spec_timeout: "abc"
}
}
let args = {
specTimeout: 20
};
isSpecTimeoutArgPassedStub.returns(true);
utils.setSpecTimeout(bsConfig, args);
expect(bsConfig.run_settings.spec_timeout).to.eq(20);
});

it('sets spec_timeout undefined if no value passed in args', () => {
let bsConfig = {
run_settings: {
spec_timeout: "abc"
}
}
let args = {};
isSpecTimeoutArgPassedStub.returns(true);
utils.setSpecTimeout(bsConfig, args);
expect(bsConfig.run_settings.spec_timeout).to.eq('undefined');
});

it('sets spec_timeout to value passed in bsConfig is not in args', () => {
let bsConfig = {
run_settings: {
spec_timeout: 20
}
}
let args = {};
isSpecTimeoutArgPassedStub.returns(false);
utils.setSpecTimeout(bsConfig, args);
expect(bsConfig.run_settings.spec_timeout).to.eq(20);
});

it('sets spec_timeout to null if no value passed in args or bsConfig', () => {
let bsConfig = {
run_settings: {}
}
let args = {};
isSpecTimeoutArgPassedStub.returns(false);
utils.setSpecTimeout(bsConfig, args);
expect(bsConfig.run_settings.spec_timeout).to.eq(null);
});
});

describe('#isInteger', () => {
it('returns true if positive integer', () => {
expect(utils.isInteger(123)).to.eq(true);
});

it('returns true if negative integer', () => {
expect(utils.isInteger(-123)).to.eq(true);
});

it('returns false if string', () => {
expect(utils.isInteger("123")).to.eq(false);
});
});

describe('#isPositiveInteger', () => {
it('returns true if string positive integer', () => {
expect(utils.isPositiveInteger("123")).to.eq(true);
});

it('returns false if string negative integer', () => {
expect(utils.isPositiveInteger("-123")).to.eq(false);
});

it('returns false if complex string without integer', () => {
expect(utils.isPositiveInteger("abc qskbd wie")).to.eq(false);
});

it('returns false if complex string with integer', () => {
expect(utils.isPositiveInteger("123 2138 a1bc qs3kbd wie")).to.eq(false);
});
});

describe('formatRequest', () => {
it('should return correct JSON', () => {
expect(utils.formatRequest('Something went wrong.', undefined, undefined)).to.be.eql({err: 'Something went wrong.', status: null, body: null});
Expand Down