appfolio / ae_page_objects

Page Objects for Capybara
MIT License
28 stars 9 forks source link

Allow missing elements to be inspected #187

Open tlconnor opened 8 years ago

tlconnor commented 8 years ago

When assert_predicate assertions fail Minitest will inspect the target object as part of the error message generation.

Currently AePageObjects will pass the inspect call on to the Element which will attempt to load the DOM element and will result in AePageObjects::LoadingElementFailed being thrown.

This PR allow inspect to be called on ElementProxy objects where the DOM element is absent without throwing an error.

rmacklin commented 8 years ago

Can you add tests? I tried to reproduce this in an existing selenium test for a simple project and wasn't been able to, so I think there is more to it than what was described above.

tlconnor commented 8 years ago

@rmacklin you are right, the root cause of the flakiness was something else. The fix I've made is still valid as the failing inspect call would hide the real error.

I've added unit tests for the fix.

rmacklin commented 8 years ago

I'm still not totally clear on the root issue here. But if this is limited to Minitest::Assertions#assert_predicate assertions, did you see this comment in the file you linked in the original PR description (before you edited it)? Seems like overriding that method in a minitest extensions module would be smaller in scope than globally changing how inspect and to_s behave, which is technically a breaking change.

dtognazzini commented 6 years ago

Seems like a pretty good improvement. I think things would become even cleaner if we collapsed ElementProxy into Element altogether: https://github.com/appfolio/ae_page_objects/issues/127#issuecomment-106092730