Skip to content
This repository was archived by the owner on Sep 21, 2022. It is now read-only.

Allow some capture elements to be missing#282

Closed
fenduru wants to merge 1 commit intogemini-testing:masterfrom
fenduru:no-error-when-missing-capture-element
Closed

Allow some capture elements to be missing#282
fenduru wants to merge 1 commit intogemini-testing:masterfrom
fenduru:no-error-when-missing-capture-element

Conversation

@fenduru
Copy link

@fenduru fenduru commented Nov 2, 2015

Previously Gemini would through an error if any of the selectors
passed to setCaptureElement were non-existent. This changes the behavior
to only throw an error if all of the selectors are non-existent.

fixes #231

@levonet levonet added the review label Nov 2, 2015
@fenduru fenduru mentioned this pull request Nov 3, 2015
Previously Gemini would through an error if any of the selectors
passed to setCaptureElement were non-existent. This changes the behavior
to only throw an error if all of the selectors are non-existent.
@fenduru fenduru force-pushed the no-error-when-missing-capture-element branch from 130df9b to ac91107 Compare November 3, 2015 23:40
@SwinX
Copy link
Contributor

SwinX commented Nov 5, 2015

@fenduru unfortunately I can't merge this because it's breaking documented behavior.
Missing selector is a critical issue indicating your test or code has some errors. You have to investigate it rather then modifying testing tool to somehow circumvent it.

Looking at #231 it looks like you just need to define custom suite for case when your popup is not opened yet.

@fenduru
Copy link
Author

fenduru commented Nov 5, 2015

The way the docs are currently written, I think this change is actually fixing a bug in the implementation.

All tests in a suite will fail if none of the elements will be found.

I would prefer that you reconsider this PR, but if you don't want to then I think the docs should be updated.

If you don't want to reconsider, what would you think about adding an additional function called setOptionalCaptureElements? The library that i'm testing uses AngularJS, where it is very common for elements to come and go from the DOM (usually via ng-if). One of the benefits of Gemini for me is not needing to do a page reload between captures when testing a complete user interaction flow (we have thousands of images, and page loads are one of the main bottlenecks)

@fenduru
Copy link
Author

fenduru commented Nov 16, 2015

@SwinX thoughts?

@SwinX
Copy link
Contributor

SwinX commented Nov 19, 2015

Seems like you just need to prepare code under test in a way it would be stable and will produce same layout all the time.
About setOptionalCaptureElements - I don't really understand, how in this case Gemini should act. If all optional and missing then test is ok? Strange logic.

@fenduru
Copy link
Author

fenduru commented Nov 19, 2015

There is no way for me to have the code being tested always produce the same layout because of the way Angular.Js works.

I agree that the "all capture elements are optional and missing" is a weird case.

I'll just work around this by having separate suites and incur the extra page loads for these tests.

@SwinX
Copy link
Contributor

SwinX commented Nov 20, 2015

@fenduru, yes, having separate suites for each case is right way for testing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Don't throw error if some capture elements are missing

3 participants