-
Notifications
You must be signed in to change notification settings - Fork 37
[terra-functional-testing] Updated specPath in BaseCompare. #644
Conversation
@@ -105,7 +105,7 @@ class BaseCompare { | |||
|
|||
// Added to allow for test reusability from terra repositories | |||
if (specPath.includes('node_modules')) { | |||
[, specPath] = specPath.split('node_modules'); | |||
specPath = specPath.replace('node_modules', 'tests/wdio/__snapshots__'); |
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.
Should we use path.join
to handle platform specific separator?
E.g. : path.join('test, 'wdio', '__snapshots__')
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.
updated here 1f7eecf
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.
let's also add a verification pull request for the clinical theme that pulls this in
@yuderekyu, You can still view the structure in which the snapshots are getting generated. |
@@ -105,7 +105,7 @@ class BaseCompare { | |||
|
|||
// Added to allow for test reusability from terra repositories | |||
if (specPath.includes('node_modules')) { | |||
[, specPath] = specPath.split('node_modules'); | |||
specPath = specPath.replace('node_modules', path.join('tests', 'wdio', '__snapshots__')); |
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.
Lets remove the snapshots path to avoid adding snapshots to the path twice.
specPath = specPath.replace('node_modules', path.join('tests', 'wdio', '__snapshots__')); | |
specPath = specPath.replace('node_modules', path.join('tests', 'wdio')); |
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.
Updated here b4c3235
@@ -105,7 +105,7 @@ class BaseCompare { | |||
|
|||
// Added to allow for test reusability from terra repositories | |||
if (specPath.includes('node_modules')) { | |||
[, specPath] = specPath.split('node_modules'); | |||
specPath = specPath.replace('node_modules', path.join('tests', 'wdio')); |
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.
since we are no longer using Array.split() can we change specPath definition to
let specPath;
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 think we can remove this.baseScreenshotDir
as that is being used to add process.cwd()
, which we are removing at the specPath
definition using Array.split()
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 was doing the pretty much the same thing as in terra-toolkit-boneyard. I wonder where the difference is between the two.
Summary
This PR replace
node_modules
in specPath in BaseCompare withtests/wdio/__snapshots__
.Closes #643
Additional Details
This is required as all the specs in clinical-theme are present in node_modules, And upgrading it to terra-functional-testing causes the snapshots to be created in the root directory.
@cerner/terra