Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix for #94 #99

Merged
merged 1 commit into from
Jun 13, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -50,26 +50,29 @@ public HtmlElementDecorator(CustomElementLocatorFactory factory) {
}

public Object decorate(ClassLoader loader, Field field) {
if (isTypifiedElement(field)) {
return decorateTypifiedElement(loader, field);
try {
Copy link
Contributor

@lanwen lanwen Jun 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you only add try-catch without changing the if style to elif? Previous variant is more readable for me

if (isTypifiedElement(field)) {
return decorateTypifiedElement(loader, field);
}
if (isHtmlElement(field)) {
return decorateHtmlElement(loader, field);
}
if (isWebElement(field) && !field.getName().equals("wrappedElement")) {
return decorateWebElement(loader, field);
}
if (isTypifiedElementList(field)) {
return decorateTypifiedElementList(loader, field);
}
if (isHtmlElementList(field)) {
return decorateHtmlElementList(loader, field);
}
if (isWebElementList(field)) {
return decorateWebElementList(loader, field);
}
return null;
} catch (ClassCastException ignore) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CRITICAL Either log or rethrow this exception. rule

return null; // See bug #94 and NonElementFieldsTest
}
if (isHtmlElement(field)) {
return decorateHtmlElement(loader, field);
}
if (isWebElement(field) && !field.getName().equals("wrappedElement")) {
return decorateWebElement(loader, field);
}
if (isTypifiedElementList(field)) {
return decorateTypifiedElementList(loader, field);
}
if (isHtmlElementList(field)) {
return decorateHtmlElementList(loader, field);
}
if (isWebElementList(field)) {
return decorateWebElementList(loader, field);
}

return null;
}

protected <T extends TypifiedElement> T decorateTypifiedElement(ClassLoader loader, Field field) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package ru.yandex.qatools.htmlelements;

import org.junit.Test;
import ru.yandex.qatools.htmlelements.testpages.NonElementFieldsPage;

import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.Matchers.emptyCollectionOf;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertThat;

/**
* @author Graham Russell [email protected]
* Date: 24.10.2015
*/
public class NonElementFieldsTest {

@Test
public void nonElementPrivateFieldsShouldNotBeDecoratedAsElements() {

NonElementFieldsPage page = new NonElementFieldsPage();

assertThat("Non-WebElement/HtmlElement fields are not touched",
page.rowsCache, is(nullValue()));
assertThat("Non-WebElement/HtmlElement fields are not touched",
page.rowsAsStringCache, is(nullValue()));
assertThat("Non-WebElement/HtmlElement fields are not touched",
page.otherCache, is(emptyCollectionOf(String.class)));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package ru.yandex.qatools.htmlelements.testpages;

import org.openqa.selenium.WebDriver;
import org.openqa.selenium.WebElement;
import ru.yandex.qatools.htmlelements.loader.HtmlElementLoader;

import java.util.ArrayList;
import java.util.List;

import static org.mockito.Mockito.mock;

/**
* @author Graham Russell [email protected]
* Date: 24.10.2015
*/
public class NonElementFieldsPage {

// public for ease of assertion in test
public List<List<WebElement>> rowsCache;
public List<List<String>> rowsAsStringCache;
public List<String> otherCache = new ArrayList<>();

public NonElementFieldsPage() {
this(mockDriver());
}

public NonElementFieldsPage(WebDriver driver) {
HtmlElementLoader.populatePageObject(this, driver);
}

public static WebDriver mockDriver() {
return mock(WebDriver.class);
}
}