Skip to content
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

Set unique screenshot name for failed tests #299

Merged
merged 33 commits into from
Dec 14, 2016
Merged

Set unique screenshot name for failed tests #299

merged 33 commits into from
Dec 14, 2016

Conversation

APshenkin
Copy link
Collaborator

Hi!
During working I was faced with two issues with failed screenshots:

  1. Scenario name can't be so long as I want, because the failed screenshot name will be also very long and will not open correctly
  2. I can't use the same scenario names in different suites (files), because if they failed, then screenshot will be overrided.

So I add new option for WebdriverIO to use hashCode of title of scenario and file name for screenshot name

@DavertMik
Copy link
Contributor

Can you provide an example of hashed filename?
Also will it be recognizable to which test does screenshot belong?

@APshenkin
Copy link
Collaborator Author

Here is an example
1963136161-940277181.failed.png
There is no way to recognize to which test does screenshot belong, but if you use any html reporter You can set the same hash function to link screenshot to scenario. (I did it for mochawesome)

Please, share your thoughts, if you have any idea how we can set unique name for screenshot so that we will be able to understand to which test does screenshot belong.

@DavertMik
Copy link
Contributor

How about prepending testname? Just to know what it is

testname-1963136161-940277181.failed.png

@APshenkin
Copy link
Collaborator Author

If we will add testname in the screenshot name then this point will not be included to this feature:
Scenario name can't be so long as I want, because the failed screenshot name will be also very long and will not open correctly
Maybe we can try some workaround: wor this mode, get first 30-40 characters of testname and include them in screenshot name

@DavertMik
Copy link
Contributor

Maybe we can try some workaround: wor this mode, get first 30-40 characters of testname and include them in screenshot name

Yep. That would be ok.

if (this.options.hashCodeScreenshotsNames)
fileName = hashCode(test.title) + '-' + hashCode(test.file) + '.failed.png';
else
fileName = test.title.replace(/ /g, '_') + '.failed.png';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't it be more readable if you would use ${string interpolation} ?

Copy link
Contributor

@DavertMik DavertMik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, few more changes from me and it is ready to be merged

@@ -194,6 +194,7 @@ class WebDriverIO extends Helper {
waitForTimeout: 1000, // ms
desiredCapabilities: {},
restart: true,
hashCodeScreenshotsNames: false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

better to name it uniqueScreenshotNames or uniqueScreenshots

@@ -288,7 +288,11 @@ class WebDriverIO extends Helper {
}

_failed(test) {
let fileName = test.title.replace(/ /g, '_') + '.failed.png';
let fileName = '';
if (this.options.hashCodeScreenshotsNames)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{ } - are important in if

@@ -1216,4 +1220,16 @@ function withStrictLocator(locator) {
}
}

function hashCode(str) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pleease move that to lib/utils with some comments about how the hashing is done and for what purpose. Because it would be hard to understand that for later users

let fileName = test.title.replace(/ /g, '_') + '.failed.png';
let fileName = '';
if (this.options.hashCodeScreenshotsNames)
fileName = hashCode(test.title) + '-' + hashCode(test.file) + '.failed.png';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be test.title.substring(0,10) or smth like that?

@APshenkin
Copy link
Collaborator Author

@DavertMik please check the changes after review and merge them :)

@DavertMik
Copy link
Contributor

Thanks, looks good now. I will try it locally then I will release a new version

@DavertMik DavertMik merged commit fe4af1c into codeceptjs:master Dec 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants