fix #152 - modify browserstack config file path to work #222
Conversation
patrickathompson
force-pushed
the
bug/issue-152
branch
2 times, most recently
from
d3b2fcb to
ffe20fc
Compare
patrickathompson
force-pushed
the
bug/issue-152
branch
from
ffe20fc to
becd08f
Compare
patrickathompson
force-pushed
the
bug/issue-152
branch
from
f17d723 to
b9f0ff4
Compare
|
needs a test for configuration file + environment variables |
patrickathompson
force-pushed
the
bug/issue-152
branch
from
f3a0778 to
0bf3ad2
Compare
|
I realized that the |
|
Hello @patrickathompson, Thank you for your pull request. We will review it and merge it if everything looks good. |
| const configPath = path.resolve(process.env.BROWSERSTACK_CAPABILITIES_CONFIG_PATH); | ||
|
|
||
| relativeConfigPath = path.relative(__dirname, configPath); |
There was a problem hiding this comment.
Since path.resolve already returns an absolute path, converting it to a relative path is unnecessary.
| 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'; |
There was a problem hiding this comment.
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(); |
There was a problem hiding this comment.
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.
| 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) |
This change will modify the provided config file path to match the relative path of the file needed by the plugin to resolve it correctly.