Closed ennjin closed 4 years ago
Just one question, what did you have in mind for the tests?
IMHO, there is copypasta and conflicts with linter. Also, the folder structure looks unclearly.
Can you also fix the merge conflict?
Sure, no problem
IMHO, there is copypasta and conflicts with linter. Also, the folder structure looks unclear, but it's minor things.
Can you explain a little bit more about what's not clear about the folder structure? By the way, I'm now adding iOS 13 images, running the tests and release a new version
Can you explain a little bit more about what's not clear about the folder structure?
I think, we should improve like this
test/
**.spec.ts
test-setup.ts
configs/
**.config.js
images/
sauseLabs/
baseline/
browser_version/
**.png
actual/
browser_version/
**.png
diff/
browser_version/
**.png
local
baseline/
browser_version/
**.png
actual/
browser_version/
**.png
diff/
browser_version/
**.png
By the way, I'm now adding iOS 13 images, running the tests and release a new version
It's cool! Because this PR still work in progress and doesn't ready to be merged.
I think, we should improve like this
test/ **.spec.ts test-setup.ts configs/ **.config.js images/ sauseLabs/ baseline/ browser_version/ **.png actual/ browser_version/ **.png diff/ browser_version/ **.png local baseline/ browser_version/ **.png actual/ browser_version/ **.png diff/ browser_version/ **.png
The reason for having this structure has to do with the following:
actual
and diff
folder should also never be pushed because they are temporary folders, that's why I put them in a .tmp/
folder.
The baseline images are also part of the tests, that's why I put them into the test folderWhat are your reasons/thoughts for wanting to change them?
the local baseline folder should never be pushed, this is for local testing only and can differ per machine
We can easily add it to .gitignore
file, the same for actual
and diff
What are your reasons/thoughts for wanting to change them?
It's just point of my view :) I think it's looks logical. What do you think about it?
the local baseline folder should never be pushed, this is for local testing only and can differ per machine
We can easily add it to
.gitignore
file, the same foractual
anddiff
What are your reasons/thoughts for wanting to change them?
It's just point of my view :) I think it's looks logical. What do you think about it?
That's true, then there should be a cleaning script before the instances start to clean the actual|diff
folders, now it's pretty easy to do that by removing the .tmp/
folder.
I would still prefer the images
to be in the test folder because it's part of the tests. For the rest I would say "Go with the flow, when you have time 😉 "
Hey, @wswebcreation, I'm done with this PR.
The changes:
Update structure of test folder
Get rid of redundant e2e tests ( I think it should be simple, like KISS ) If library can compare images without errors then it work correctly and we don't need to check path or do complex stuff
Now, all baselines saving into images
folder. Actual
and diff
folders automatically removed before run.
I checked and created baselines only for desktop chrome browser. It will be nice if you run it in saucelabs and push new images into my branch.
From my point of view, now it's look neatly :)
Small note:
If needed to cover libraries methods you should create unit test instead of e2e. Also, I think, it should be in webdriver-image-comparison
package.
Thanks for all the work, I'll take a look at it and recreate a baseline the coming days. It will take me some time.
Updated! Please, ckeck it.
Thanks for the hard work @vitalie-ly , going to release a new version today
Cool :) I think version should be 3.8.1 because there isn't major changes. Anyway, thanks for you work too.
Changing some small things and hope to do a release in a few minutes
It's just suggestion for improve dev environment.
Completed:
What will need to do
It's just suggestion. You can just close this PR if you don't wanna delve into this, it's OK!