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

Fix some small bugs in error handlers for image capture #583

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

stevenday
Copy link

@stevenday stevenday commented Sep 7, 2018

A couple of small things I noticed whilst getting lots of things wrong in trying to use Wraith.

68ccfe6 fixes something I think got accidentally merged away in: 546c713#diff-b6c1420e088ed8e1127a35c6ac8ec84eL79 where you put the name of the invalid image to use into each job (presumably so that they're still different if both fail).

e0de3ed is a smaller typo, where the timeout rescue block was trying to log what screen size was being used, but had the wrong variable name.

I haven't added specs for the logging fix as I don't normally test logging, but I've written something for the invalid image creation. Let me know if the approach I've taken suits or not, as I kinda free-styled it.

Steve Day added 3 commits September 7, 2018 16:59
I think this got accidentally merged away in:
bbc@546c713#diff-b6c1420e088ed8e1127a35c6ac8ec84eL79

Without it, when we fail to capture an image, we also fail to create a
dummy one to stand in for it.
These specs pass on Windows, but fail on Travis, so try a different way
of comparing images.
@stevenday
Copy link
Author

Sorry about that, I had a few test failures here which worked fine for me locally, but failed on Travis. I wasn't sure exactly how best to test the invalid image was getting created properly, but I've eventually found a way that's reliable on both - using Wraith! (I copied some other tests, so hopefully this approach is acceptable). I've pushed the changes as a fixup in 9a3a247 in case you'd already looked at the old version.

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.

1 participant