Merge pull request #315 from zanata/fix-ft-findbugs

Fix findbugs warnings in functional test framework

functional-test/pom.xml

@@ -306,6 +306,10 @@
       <scope>test</scope>
     </dependency>
 
+    <dependency>
+      <groupId>com.google.code.findbugs</groupId>
+      <artifactId>jsr305</artifactId>
+    </dependency>
 
   </dependencies>
 

functional-test/src/main/java/org/zanata/page/WebDriverFactory.java

@@ -47,7 +47,7 @@
 public enum WebDriverFactory {
     INSTANCE;
 
-    private WebDriver driver;
+    private volatile WebDriver driver;
     private DriverService driverService;
 
     public WebDriver getDriver() {

functional-test/src/main/java/org/zanata/page/account/RegisterPage.java

@@ -108,12 +108,11 @@ public RegisterPage clearFields() {
      * Fields: name, email, username, password, confirmpassword, captcha
      */
     public RegisterPage setFields(Map<String, String> fields) {
-        clearFields();
-        enterName(fields.get("name"));
-        enterEmail(fields.get("email"));
-        enterUserName(fields.get("username"));
-        enterPassword(fields.get("password"));
-        return new RegisterPage(getDriver());
+        return clearFields()
+            .enterName(fields.get("name"))
+            .enterEmail(fields.get("email"))
+            .enterUserName(fields.get("username"))
+            .enterPassword(fields.get("password"));
     }
 
     public List<String> waitForErrors() {

functional-test/src/main/java/org/zanata/util/PropertiesHolder.java

@@ -12,7 +12,7 @@
  */
 @Slf4j
 public class PropertiesHolder {
-    public static Properties properties;
+    public final static Properties properties;
 
     static {
         Properties result;

functional-test/src/main/java/org/zanata/util/TestFileGenerator.java

@@ -21,7 +21,9 @@
 package org.zanata.util;
 
 import org.apache.commons.io.FileUtils;
+
 import java.io.*;
+import java.nio.charset.Charset;
 
 /**
  * Create and manipulate basic text files for testing.
@@ -82,10 +84,12 @@ private File generateTestFile(String fileName, String suffix) {
 
     private void setTestFileContent(File testFile, String testContent) {
         try {
-            FileWriter fileWriter = new FileWriter(testFile);
-            fileWriter.write(testContent);
-            fileWriter.flush();
-            fileWriter.close();
+            OutputStreamWriter outputStreamWriter = new OutputStreamWriter(
+                    new FileOutputStream(testFile),
+                    Charset.forName("UTF-8").newEncoder());
+            outputStreamWriter.write(testContent);
+            outputStreamWriter.flush();
+            outputStreamWriter.close();
         } catch (IOException ioException) {
             throw new RuntimeException("Could not open file for writing "
                     + testFile.getName());

functional-test/src/main/java/org/zanata/util/WebElementUtil.java

@@ -24,6 +24,8 @@
 import java.util.Collections;
 import java.util.List;
 
+import javax.annotation.Nullable;
+
 import com.google.common.base.Function;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Collections2;
@@ -65,7 +67,10 @@ public static List<TableRow> getTableRows(WebDriver driver,
         return waitForTenSeconds(driver).until(
                 new Function<WebDriver, List<TableRow>>() {
                     @Override
-                    public List<TableRow> apply(WebDriver webDriver) {
+                    public List<TableRow> apply(@Nullable WebDriver webDriver) {
+                        if (webDriver == null) {
+                            throw new RuntimeException("Driver is null");
+                        }
                         final WebElement table =
                                 webDriver.findElement(byQueryForTable);
                         List<WebElement> rows =
@@ -81,7 +86,10 @@ public static List<TableRow> getTableRows(WebDriver driver,
         return waitForTenSeconds(driver).until(
                 new Function<WebDriver, List<TableRow>>() {
                     @Override
-                    public List<TableRow> apply(WebDriver webDriver) {
+                    public List<TableRow> apply(@Nullable WebDriver webDriver) {
+                        if (webDriver == null) {
+                            throw new RuntimeException("Driver is null");
+                        }
                         List<WebElement> rows =
                                 table.findElements(By.xpath(".//tbody[1]/tr"));
                         return ImmutableList.copyOf(Lists.transform(rows,
@@ -95,7 +103,10 @@ public static ImmutableList<List<String>> transformToTwoDimensionList(
         return ImmutableList.copyOf(Lists.transform(tableRows,
                 new Function<TableRow, List<String>>() {
                     @Override
-                    public List<String> apply(TableRow from) {
+                    public List<String> apply(@Nullable TableRow from) {
+                        if (from == null) {
+                            throw new RuntimeException("Source table is null");
+                        }
                         return from.getCellContents();
                     }
                 }));
@@ -119,7 +130,10 @@ public static List<String> getColumnContents(WebDriver driver, final By by,
         return waitForTenSeconds(driver).until(
                 new Function<WebDriver, List<String>>() {
                     @Override
-                    public List<String> apply(WebDriver input) {
+                    public List<String> apply(@Nullable WebDriver input) {
+                        if (input == null) {
+                            throw new RuntimeException("Driver is null");
+                        }
                         WebElement table;
                         try {
                             table = input.findElement(by);
@@ -156,7 +170,10 @@ public static List<List<String>> getTwoDimensionList(WebDriver driver,
         return waitForTenSeconds(driver).until(
                 new Function<WebDriver, List<List<String>>>() {
                     @Override
-                    public List<List<String>> apply(WebDriver input) {
+                    public List<List<String>> apply(@Nullable WebDriver input) {
+                        if (input == null) {
+                            throw new RuntimeException("Driver is null");
+                        }
                         final WebElement table = input.findElement(by);
                         List<WebElement> rows =
                                 table.findElements(By.xpath(".//tbody[1]/tr"));
@@ -172,7 +189,10 @@ public static List<WebElement> getListItems(WebDriver driver, final By by) {
         return waitForTenSeconds(driver).until(
                 new Function<WebDriver, List<WebElement>>() {
                     @Override
-                    public List<WebElement> apply(WebDriver input) {
+                    public List<WebElement> apply(@Nullable WebDriver input) {
+                        if (input == null) {
+                            throw new RuntimeException("Driver is null");
+                        }
                         final WebElement list = input.findElement(by);
                         return list.findElements(By.xpath(".//li"));
                     }
@@ -209,7 +229,10 @@ public static enum WebElementToTextFunction implements
         FUNCTION;
 
         @Override
-        public String apply(WebElement from) {
+        public String apply(@Nullable WebElement from) {
+            if (from == null) {
+                throw new RuntimeException("Source element is null");
+            }
             return from.getText();
         }
     }

functional-test/src/main/java/org/zanata/workflow/LanguageWorkFlow.java

@@ -30,15 +30,14 @@
 @Slf4j
 public class LanguageWorkFlow extends AbstractWebWorkFlow {
 
-    public void addLanguageAndJoin(String localeId) {
-        ManageLanguagePage manageLanguagePage = addLanguage(localeId);
-        ManageLanguageTeamMemberPage teamMemberPage =
-                manageLanguagePage.manageTeamMembersFor(localeId);
-        if (!teamMemberPage.getMemberUsernames().contains("admin")) {
-            teamMemberPage.joinLanguageTeam();
-        } else {
+    public ManageLanguageTeamMemberPage addLanguageAndJoin(String localeId) {
+        ManageLanguageTeamMemberPage teamMemberPage = addLanguage(localeId)
+                .manageTeamMembersFor(localeId);
+        if (teamMemberPage.getMemberUsernames().contains("admin")) {
             log.warn("admin has already joined the language [{}]", localeId);
+            return teamMemberPage;
         }
+        return teamMemberPage.joinLanguageTeam();
     }
 
     public ManageLanguagePage addLanguage(String localeId) {

functional-test/src/main/java/org/zanata/workflow/LoginWorkFlow.java

@@ -60,23 +60,23 @@ public DashboardPage signIn(String username, String password) {
     }
 
     public SignInPage signInFailure(String username, String password) {
-        SignInPage signInPage = new BasePage(driver).clickSignInLink();
         log.info("log in as username: {}", username);
-        signInPage.enterUsername(username);
-        signInPage.enterPassword(password);
-        signInPage.clickSignInExpectError();
-        return new SignInPage(driver);
+        return new BasePage(driver)
+            .clickSignInLink()
+            .enterUsername(username)
+            .enterPassword(password)
+            .clickSignInExpectError();
     }
 
     private void doSignIn(String username, String password) {
+        log.info("log in as username: {}", username);
         BasePage basePage = new BasePage(driver);
         basePage.deleteCookiesAndRefresh();
-        SignInPage signInPage = basePage.clickSignInLink();
-        log.info("log in as username: {}", username);
-        signInPage.enterUsername(username);
-        signInPage.enterPassword(password);
-        signInPage.clickSignIn();
-        signInPage.waitForTenSec().until(new Predicate<WebDriver>() {
+        basePage.clickSignInLink()
+            .enterUsername(username)
+            .enterPassword(password)
+            .clickSignIn()
+            .waitForTenSec().until(new Predicate<WebDriver>() {
             @Override
             public boolean apply(WebDriver driver) {
                 List<WebElement> messages =

functional-test/src/test/java/org/zanata/feature/security/SecurityFullTest.java

@@ -63,9 +63,10 @@ public void signInSuccessful() {
     }
 
     @Test
+    @Category(BasicAcceptanceTest.class)
     public void signInFailure() {
-        SignInPage signInPage =
-                new LoginWorkFlow().signInFailure("nosuchuser", "password");
+        SignInPage signInPage = new LoginWorkFlow()
+                .signInFailure("nosuchuser", "password");
 
         assertThat("Error message is shown",
                 signInPage.waitForFieldErrors(),