Fix: Double Ally testcase save calls #988
Conversation
Sdk 3449 new2
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
bin/helpers/helper.js
Outdated
| @@ -390,7 +390,7 @@ exports.getSupportFiles = (bsConfig, isA11y) => { | |||
| supportFile = userSupportFile[0]; | |||
| } | |||
| } catch (err) {} | |||
| if(supportFile && supportFile[0] != '/') supportFile = '/' + supportFile; | |||
| if (supportFile && !path.isAbsolute(supportFile)) supportFile = '/' + supportFile; | |||
There was a problem hiding this comment.
Why is this change needed? What other places do we use this?
There was a problem hiding this comment.
@karanshah-browserstack this change was done to check if the given path is absolute or not with in-built function in path module
There was a problem hiding this comment.
@Bhargavi-BS Was this failing in any scenario for you guys?
There was a problem hiding this comment.
It had failed once for single path being sent in supportFile - not sure of the reason but then co-pilot had suggested to use this method instead and it passed in the next run
There was a problem hiding this comment.
In Windows, ../../karan/project/support.js will return me a absolute path false.
supportFile = /../../karan/project/support.js.
The above condition, is currently handling linux & mac paths imo. Your change will cause issue in Windows. Let's not go ahead with the above change.
| glob(process.cwd() + supportFilesData.supportFile, {}, (err, files) => { | ||
| if(err) return logger.debug('EXCEPTION IN BUILD START EVENT : Unable to parse cypress support files'); | ||
|
|
||
| const isPattern = glob.hasMagic(supportFilesData.supportFile); |
There was a problem hiding this comment.
Can you explain the changes done as part of this?
There was a problem hiding this comment.
@karanshah-browserstack this is to check if the returned supportFile is a single file path or a glob pattern cause if it's a single path then user has explicitly passed in their cypress config and plugins should be added to only that file .If glob pattern is present which we return by default and then that needs to be treated with a loop.
bin/helpers/helper.js
Outdated
| @@ -390,7 +390,7 @@ exports.getSupportFiles = (bsConfig, isA11y) => { | |||
| supportFile = userSupportFile[0]; | |||
| } | |||
| } catch (err) {} | |||
| if(supportFile && supportFile[0] != '/') supportFile = '/' + supportFile; | |||
| if (supportFile && !path.isAbsolute(supportFile)) supportFile = '/' + supportFile; | |||
There was a problem hiding this comment.
In Windows, ../../karan/project/support.js will return me a absolute path false.
supportFile = /../../karan/project/support.js.
The above condition, is currently handling linux & mac paths imo. Your change will cause issue in Windows. Let's not go ahead with the above change.
|
|
||
| const isPattern = glob.hasMagic(supportFilesData.supportFile); | ||
| if(!isPattern) { | ||
| console.log(`Using user defined support file: ${supportFilesData.supportFile}`); |
There was a problem hiding this comment.
Move this to logger.
| console.log(`Using user defined support file: ${supportFilesData.supportFile}`); | ||
| let file; | ||
| try { | ||
| file = process.cwd() + supportFilesData.supportFile; |
There was a problem hiding this comment.
Should you check whether supportfile is absolute/relative path? If we are always getting an absolute path why do we cwd to be added to it?
There was a problem hiding this comment.
Test it windows + mac, locally.
@karanshah-browserstack - Sanity as in the run cypress tests on both MAC and Windows right ? Can you please clarify here. |
No description provided.