search

mattheworiordan / capybara-screenshot

Automatically save screen shots when a Capybara scenario fails
MIT License
1.02k stars 168 forks source link

take screenshots of correct window #245

Closed dreyks closed 2 years ago

dreyks commented 5 years ago

fixes #143

unfortunately couldn't figure out how to properly test this - base capybara driver is not capable of switching windows

mattheworiordan commented 5 years ago

Thanks for this contribution @dreyks, however this code is not threadsafe AFAICT. Capybara Screenshot does indeed share static variables, however unless there is a bug, the idea is that session (test) specific state is not stored at a static class level for obvious reasons (parallel tests). Would you be able to look into a way to use a test local variable instead?

dreyks commented 5 years ago

🤔 At first I've been thinking about that but then I saw some other places of direct writing into a static variable like this https://github.com/mattheworiordan/capybara-screenshot/blob/cac1beedff40a5fb7cf46853b35f2c97f5a1eafd/lib/capybara-screenshot/capybara.rb#L18

But I think I can store the offending window in the capybara session (or page) itself

mattheworiordan commented 5 years ago

At first I've been thinking about that but then I saw some other places of direct writing into a static variable like this

Mmm, that's not good. I'll raise an issue to fix that too down the line.

trcarden commented 5 years ago

@mattheworiordan and @dreyks Has this stalled out? I found myself also in a situation where it would be quite nice to the the screenshot from the new window.

mattheworiordan commented 5 years ago

Has this stalled out?

Yup, waiting on @dreyks to update I am afraid.

cgunther commented 3 years ago

@trcarden check out #287

mattheworiordan commented 2 years ago

Closing as superseded by https://github.com/mattheworiordan/capybara-screenshot/pull/287