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.

Sign up for GitHub

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

fix #152 - modify browserstack config file path to work #222

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix #152 - modify browserstack config file path to work #222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
becd08f
fix #152 - modify browserstack config file path to work
patrickathompson Aug 5, 2024
b9f0ff4
fix test errors
patrickathompson Aug 5, 2024
0bf3ad2
added test and refactored
patrickathompson Aug 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { parse as parseUrl } from 'url';
import path from 'node:path';
import Promise from 'pinkie';
import { promisify } from 'util';
import parseCapabilities from 'desired-capabilities';
Expand Down Expand Up @@ -137,12 +138,17 @@ module.exports = {
},

_getCapabilitiesFromConfig () {
const configPath = process.env.BROWSERSTACK_CAPABILITIES_CONFIG_PATH;
let relativeConfigPath;

if (!configPath)
if (process.env.BROWSERSTACK_CAPABILITIES_CONFIG_PATH) {
const configPath = path.resolve(process.env.BROWSERSTACK_CAPABILITIES_CONFIG_PATH);

relativeConfigPath = path.relative(__dirname, configPath);
Comment on lines +144 to +146
Copy link
Contributor

Choose a reason for hiding this comment

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

Since path.resolve already returns an absolute path, converting it to a relative path is unnecessary.

}
else
return {};

return require(configPath);
return require(relativeConfigPath);
},

_getAdditionalCapabilities () {
Expand Down
34 changes: 34 additions & 0 deletions test/mocha/browserstack-capabilities-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,38 @@ describe('Browserstack capabilities', function () {
'browserstack.networkProfile': '4g-lte-lossy'
});
});

it('Should add additional capabilities when both config file and environment variables used', () => {
process.env['BROWSERSTACK_BUILD_ID'] = 'build-1';
process.env['BROWSERSTACK_PROJECT_NAME'] = 'project-1';
process.env['BROWSERSTACK_DISPLAY_RESOLUTION'] = '1024x768';
process.env['BROWSERSTACK_TEST_RUN_NAME'] = 'Testcafe test run 1';
process.env['BROWSERSTACK_DEBUG'] = 'false'; //env var should take precedence
process.env['BROWSERSTACK_CONSOLE'] = 'errors';
process.env['BROWSERSTACK_NETWORK_LOGS'] = 'true';
process.env['BROWSERSTACK_VIDEO'] = 'true';
process.env['BROWSERSTACK_TIMEZONE'] = 'Asia/Taipei';
process.env['BROWSERSTACK_GEO_LOCATION'] = 'ZA';
process.env['BROWSERSTACK_CUSTOM_NETWORK'] = '"1000", "1000", "100", "1"';
process.env['BROWSERSTACK_NETWORK_PROFILE'] = '4g-lte-lossy';
Comment on lines +68 to +79
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the test uses a configuration file and environment variables, it is not necessary to duplicate all variables present in the configuration file. It is sufficient to set just one variable that differs from the one in the configuration.


process.env.BROWSERSTACK_CAPABILITIES_CONFIG_PATH = require.resolve('./data/capabilities-config.json');

const capabilities = browserStackProvider._getAdditionalCapabilities();
Comment on lines +81 to +83
Copy link
Contributor

Choose a reason for hiding this comment

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

The current implementation sets an absolute path, but we need to test the functionality of _getCapabilitiesFromConfig with a relative path.

When a relative path is passed to path.resolve() in _getCapabilitiesFromConfig, it returns an absolute path based on the working directory. Therefore, I suggest you save the current working directory, set __dirname before retrieving the capabilities, and then restore the original working directory afterwards.

Suggested change
process.env.BROWSERSTACK_CAPABILITIES_CONFIG_PATH = require.resolve('./data/capabilities-config.json');
const capabilities = browserStackProvider._getAdditionalCapabilities();
process.env.BROWSERSTACK_CAPABILITIES_CONFIG_PATH = './data/capabilities-config.json';
const originWorkDir = process.cwd();
process.chdir(__dirname)
const capabilities = browserStackProvider._getAdditionalCapabilities();
process.chdir(originWorkDir)


expect(capabilities).to.deep.equal({
'build': 'build-1',
'project': 'project-1',
'resolution': '1024x768',
'name': 'Testcafe test run 1',
'browserstack.debug': 'false', //env var should take precedence
'browserstack.console': 'errors',
'browserstack.networkLogs': 'true',
'browserstack.video': 'true',
'browserstack.timezone': 'Asia/Taipei',
'browserstack.geoLocation': 'ZA',
'browserstack.customNetwork': '"1000", "1000", "100", "1"',
'browserstack.networkProfile': '4g-lte-lossy'
});
});
});
Loading