-
Notifications
You must be signed in to change notification settings - Fork 15
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
feat: support browser args #278
base: main
Are you sure you want to change the base?
feat: support browser args #278
Conversation
cb06cd2
to
1bb2f3b
Compare
Resolved the comments since none of the original code from my first pass exists anymore |
@@ -19,6 +20,9 @@ const { defaults } = require('@faltest/utils'); | |||
// We aren't using `@wdio/cli` (wdio testrunner) | |||
process.env.SUPPRESS_NO_CONFIG_WARNING = 'true'; | |||
|
|||
let configDir = process.env.FALTEST_CONFIG_DIR || process.cwd(); | |||
|
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.
if interested, I could do another PR for adding prettier
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.
I wouldn't want this project to have a custom solution. I'd want everything to be in it's own package like https://github.com/CrowdStrike/eslint-config-crowdstrike, then appllied here and in other repos.
|
||
let faltestConfig = loadConfig(); | ||
|
||
let argsFromConfig = (((faltestConfig.options || {}).browsers || {})[_browser] || {}).args || []; |
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.
this is what my .faltestrc.js
looks like
'use strict';
module.exports = {
options: {
targets: {
default: 'default',
list: ['default', 'local', 'ember', 'pull-request'],
},
browsers: {
firefox: {
args: [
// TODO
],
},
chrome: {
args: [
'--window-size=1280,720', // 720p
'--ignore-certificate-errors'
].filter(Boolean)
}
}
},
globs: ['tests/**/*-test.js'],
};
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.
I'm not sure about this solution. The remote
package should not know anything about the faltest config, because it is a standalone package. Only the cli
package is responsible for dealing with the config.
@@ -19,6 +20,9 @@ const { defaults } = require('@faltest/utils'); | |||
// We aren't using `@wdio/cli` (wdio testrunner) | |||
process.env.SUPPRESS_NO_CONFIG_WARNING = 'true'; | |||
|
|||
let configDir = process.env.FALTEST_CONFIG_DIR || process.cwd(); | |||
|
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.
I wouldn't want this project to have a custom solution. I'd want everything to be in it's own package like https://github.com/CrowdStrike/eslint-config-crowdstrike, then appllied here and in other repos.
I have no idea how to test this locally -- I tried linking, but for one minute, the browsers just repeatedly open and close. :-\