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

Adding more information about failed tests #297

Merged
merged 4 commits into from
Nov 5, 2015

Conversation

marciovicente
Copy link
Contributor

As developer running wraith in continuos integration environment, I need more information about which tests have failed. So, I change a little bit the code to generated this log in console.

I know this code increase the complexity of code, but I think it's very important.

Look the screenshot:
image

@marciovicente
Copy link
Contributor Author

@ChrisBAshton could you take a look here? 😁

@ChrisBAshton
Copy link
Contributor

Hi @marciovicente, neat idea! What's the most useful feedback we can give to the developer? I think something like this:

Failures detected:
    buttons failed at a resolution of 320 (4.14% diff)
    buttons failed at a resolution of 624x768 (30% diff)

Steps:

  1. Add ':' to end of 'Failures detected'
  2. Add a few spaces before listing out the failed item
  3. This also needs to support cases where the height has been set manually (see the second failure in my example above)
  4. Put the % diff in brackets after the message
  5. The red color is nice but I think \e[31m is Unix-only. We have to try to support Linux, OSX and Windows, so I'd be inclined to remove it.

Do you think you could tweak your PR to do this? That would be great!

@marciovicente
Copy link
Contributor Author

@ChrisBAshton
image

Look this commit also. It's a smooth change, instead parse a string like 320x480_phantomjs to integer (returns 320), parse string by underscore and get first parameter (the resolution / returns 320x480). I think it's a better approach, once now you can pass height to screen_widths.

@marciovicente
Copy link
Contributor Author

👀

@ChrisBAshton ChrisBAshton added this to the 2.8 milestone Nov 5, 2015
@ChrisBAshton ChrisBAshton mentioned this pull request Nov 5, 2015
@ChrisBAshton ChrisBAshton merged commit 648038d into bbc:master Nov 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants