ncbi / robotframework-pageobjects

Implementation of the Page Object pattern with Robot Framework and selenium. Also facilitates page object pattern independent of Robot Framework
http://ncbi.github.io/robotframework-pageobjects
Other
84 stars 75 forks source link

Fix for attempted screenshots in get_keyword_names #48

Closed dpsfrishberg closed 9 years ago

dpsfrishberg commented 9 years ago

Summary of the problem:

Version 1.3.0 (which was rolled back) causes a screenshot warning whenever components are used. This also breaks output.xml. This happens because get_keyword_names is called on every Page subclass when Robot loads it: https://github.com/ncbi/robotframework-pageobjects/blob/1.2.1/robotpageobjects/page.py#L209 get_keyword_names iterates over all public members of the class, including @properties. https://github.com/ncbi/robotframework-pageobjects/blob/1.2.1/robotpageobjects/base.py#L56 Retrieving a @property member from a class causes its immediate evaluation. The problem with this is that some properties (such as those automatically added to point to component instances) require an open browser (driver) before they can be evaluated. We therefore wrap the getattr call that retrieves these properties in a try/except. The problem with that is that the run-on-failure decorator still fires within the try, attempting a screenshot, which fails because there is no open browser, and the warning that it failed is still logged.

This wasn't a problem in the old version of RF-PO, because the run-on-failure keyword was set to "None", so that no screenshot was attempted within the try. It was set to "None" as a workaround for the bug where multiple screenshots were attempted when one keyword wrapped another keyword. Now that that is no longer necessary, the workaround was taken out, but that caused this bug. Blargh.

As for why the travis build passed, I suspect it's because all our tests passed, warning and busted output files or no.

This fix prevents the screenshot attempt within the aforementioned try/except by setting the {{_has_run_on_failure}} property to {{False}} (see the diff).

hellmanj commented 9 years ago

:+1:

kahunacohen commented 9 years ago

:+1: