Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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 stevenday force-pushed the fix-save-image-error-logging branch from d98966d to 9a3a247 Compare September 10, 2018 09:55
@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.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant