From 99eb71c0cc9cf7a92754b1e149c8fedb376d9600 Mon Sep 17 00:00:00 2001 From: Marty30 Date: Wed, 21 Jun 2017 16:41:04 +0200 Subject: [PATCH 1/9] Refactor the code smells that were present according to SonarQube --- .../petclinic/PetClinicApplication.java | 4 +-- .../samples/petclinic/owner/Owner.java | 6 ++--- .../petclinic/owner/OwnerController.java | 6 ++--- .../petclinic/system/CrashController.java | 3 +-- .../petclinic/system/ExampleException.java | 15 +++++++++++ .../samples/petclinic/vet/VetController.java | 8 +++--- .../samples/petclinic/vet/VetRepository.java | 2 +- .../samples/petclinic/vet/Vets.java | 8 +++--- .../petclinic/visit/VisitRepository.java | 2 +- .../nl/utwente/bpsd/selenium/FailingIT.java | 26 ------------------- 10 files changed, 34 insertions(+), 46 deletions(-) create mode 100644 src/main/java/org/springframework/samples/petclinic/system/ExampleException.java delete mode 100644 src/test/java/nl/utwente/bpsd/selenium/FailingIT.java diff --git a/src/main/java/org/springframework/samples/petclinic/PetClinicApplication.java b/src/main/java/org/springframework/samples/petclinic/PetClinicApplication.java index 224c326c7..5265770dc 100644 --- a/src/main/java/org/springframework/samples/petclinic/PetClinicApplication.java +++ b/src/main/java/org/springframework/samples/petclinic/PetClinicApplication.java @@ -21,14 +21,14 @@ import org.springframework.boot.autoconfigure.SpringBootApplication; /** * PetClinic Spring Boot Application. - * + * * @author Dave Syer * */ @SpringBootApplication public class PetClinicApplication { - public static void main(String[] args) throws Exception { + public static void main(String[] args) { SpringApplication.run(PetClinicApplication.class, args); } diff --git a/src/main/java/org/springframework/samples/petclinic/owner/Owner.java b/src/main/java/org/springframework/samples/petclinic/owner/Owner.java index f6fcae7ac..e471b5c8a 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/Owner.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/Owner.java @@ -126,13 +126,13 @@ public class Owner extends Person { * @param name to test * @return true if pet name is already in use */ - public Pet getPet(String name, boolean ignoreNew) { - name = name.toLowerCase(); + public Pet getPet(final String name, final boolean ignoreNew) { + String lowerCaseName = name.toLowerCase(); for (Pet pet : getPetsInternal()) { if (!ignoreNew || !pet.isNew()) { String compName = pet.getName(); compName = compName.toLowerCase(); - if (compName.equals(name)) { + if (compName.equals(lowerCaseName)) { return pet; } } diff --git a/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java b/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java index ef3169b70..58be460a0 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java @@ -78,7 +78,7 @@ class OwnerController { } @RequestMapping(value = "/owners", method = RequestMethod.GET) - public String processFindForm(Owner owner, BindingResult result, Map model) { + public String processFindForm(final Owner owner, final BindingResult result, final Map model) { // allow parameterless GET request for /owners to return all records if (owner.getLastName() == null) { @@ -93,8 +93,8 @@ class OwnerController { return "owners/findOwners"; } else if (results.size() == 1) { // 1 owner found - owner = results.iterator().next(); - return "redirect:/owners/" + owner.getId(); + Owner founndOwner = results.iterator().next(); + return "redirect:/owners/" + founndOwner.getId(); } else { // multiple owners found model.put("selections", results); diff --git a/src/main/java/org/springframework/samples/petclinic/system/CrashController.java b/src/main/java/org/springframework/samples/petclinic/system/CrashController.java index a702cfc8f..5fe9a3294 100644 --- a/src/main/java/org/springframework/samples/petclinic/system/CrashController.java +++ b/src/main/java/org/springframework/samples/petclinic/system/CrashController.java @@ -31,8 +31,7 @@ class CrashController { @RequestMapping(value = "/oups", method = RequestMethod.GET) public String triggerException() { - throw new RuntimeException( - "Expected: controller used to showcase what " + "happens when an exception is thrown"); + throw new ExampleException("Expected: controller used to showcase what happens when an exception is thrown"); } } diff --git a/src/main/java/org/springframework/samples/petclinic/system/ExampleException.java b/src/main/java/org/springframework/samples/petclinic/system/ExampleException.java new file mode 100644 index 000000000..bcf5e2bdd --- /dev/null +++ b/src/main/java/org/springframework/samples/petclinic/system/ExampleException.java @@ -0,0 +1,15 @@ +package org.springframework.samples.petclinic.system; + +/** + * @author Martijn + * @since 21-6-2017. + */ +public class ExampleException extends RuntimeException { + public ExampleException(String message) { + super(ExampleException.class.getSimpleName() + ": " + message); + } + + protected ExampleException(String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace) { + super(ExampleException.class.getSimpleName() + ": " + message, cause, enableSuppression, writableStackTrace); + } +} diff --git a/src/main/java/org/springframework/samples/petclinic/vet/VetController.java b/src/main/java/org/springframework/samples/petclinic/vet/VetController.java index 8ddcca60a..7911aa702 100644 --- a/src/main/java/org/springframework/samples/petclinic/vet/VetController.java +++ b/src/main/java/org/springframework/samples/petclinic/vet/VetController.java @@ -31,11 +31,11 @@ import org.springframework.web.bind.annotation.ResponseBody; @Controller class VetController { - private final VetRepository vets; + private final VetRepository vetRepository; @Autowired public VetController(VetRepository clinicService) { - this.vets = clinicService; + this.vetRepository = clinicService; } @RequestMapping(value = { "/vets.html" }) @@ -43,7 +43,7 @@ class VetController { // Here we are returning an object of type 'Vets' rather than a collection of Vet // objects so it is simpler for Object-Xml mapping Vets vets = new Vets(); - vets.getVetList().addAll(this.vets.findAll()); + vets.getVetList().addAll(this.vetRepository.findAll()); model.put("vets", vets); return "vets/vetList"; } @@ -53,7 +53,7 @@ class VetController { // Here we are returning an object of type 'Vets' rather than a collection of Vet // objects so it is simpler for JSon/Object mapping Vets vets = new Vets(); - vets.getVetList().addAll(this.vets.findAll()); + vets.getVetList().addAll(this.vetRepository.findAll()); return vets; } diff --git a/src/main/java/org/springframework/samples/petclinic/vet/VetRepository.java b/src/main/java/org/springframework/samples/petclinic/vet/VetRepository.java index 20863ce76..bbe5d5fdd 100644 --- a/src/main/java/org/springframework/samples/petclinic/vet/VetRepository.java +++ b/src/main/java/org/springframework/samples/petclinic/vet/VetRepository.java @@ -40,7 +40,7 @@ public interface VetRepository extends Repository { */ @Transactional(readOnly = true) @Cacheable("vets") - Collection findAll() throws DataAccessException; + Collection findAll(); } diff --git a/src/main/java/org/springframework/samples/petclinic/vet/Vets.java b/src/main/java/org/springframework/samples/petclinic/vet/Vets.java index f5b24c3fc..152fa560e 100644 --- a/src/main/java/org/springframework/samples/petclinic/vet/Vets.java +++ b/src/main/java/org/springframework/samples/petclinic/vet/Vets.java @@ -30,14 +30,14 @@ import javax.xml.bind.annotation.XmlRootElement; @XmlRootElement public class Vets { - private List vets; + private List vetList; @XmlElement public List getVetList() { - if (vets == null) { - vets = new ArrayList<>(); + if (vetList == null) { + vetList = new ArrayList<>(); } - return vets; + return vetList; } } diff --git a/src/main/java/org/springframework/samples/petclinic/visit/VisitRepository.java b/src/main/java/org/springframework/samples/petclinic/visit/VisitRepository.java index c7853d170..d0c8d8c41 100644 --- a/src/main/java/org/springframework/samples/petclinic/visit/VisitRepository.java +++ b/src/main/java/org/springframework/samples/petclinic/visit/VisitRepository.java @@ -38,7 +38,7 @@ public interface VisitRepository extends Repository { * @param visit the Visit to save * @see BaseEntity#isNew */ - void save(Visit visit) throws DataAccessException; + void save(Visit visit); List findByPetId(Integer petId); diff --git a/src/test/java/nl/utwente/bpsd/selenium/FailingIT.java b/src/test/java/nl/utwente/bpsd/selenium/FailingIT.java deleted file mode 100644 index 87a26e3ad..000000000 --- a/src/test/java/nl/utwente/bpsd/selenium/FailingIT.java +++ /dev/null @@ -1,26 +0,0 @@ -package nl.utwente.bpsd.selenium; - -import org.junit.Assert; -import org.junit.Ignore; -import org.junit.Test; -import org.junit.experimental.categories.Category; - -import java.net.MalformedURLException; - -/** - * @author Martijn - * @since 21-6-2017. - */ -public class FailingIT extends SeleniumBaseIT { - public FailingIT() throws MalformedURLException { - super(); - } - - @Test - @Category(SeleniumBaseIT.class) - @Ignore - public void failIT() { - Assert.fail(); - } - -} From 029fc4ec4e3e2754e523c957b33a1771eade187c Mon Sep 17 00:00:00 2001 From: Marty30 Date: Wed, 21 Jun 2017 16:41:04 +0200 Subject: [PATCH 2/9] Refactor the code smells that were present according to SonarQube --- .../petclinic/PetClinicApplication.java | 4 +-- .../samples/petclinic/owner/Owner.java | 6 ++--- .../petclinic/owner/OwnerController.java | 6 ++--- .../petclinic/system/CrashController.java | 3 +-- .../petclinic/system/ExampleException.java | 15 +++++++++++ .../samples/petclinic/vet/VetController.java | 8 +++--- .../samples/petclinic/vet/VetRepository.java | 3 +-- .../samples/petclinic/vet/Vets.java | 8 +++--- .../petclinic/visit/VisitRepository.java | 3 +-- .../nl/utwente/bpsd/selenium/FailingIT.java | 26 ------------------- 10 files changed, 34 insertions(+), 48 deletions(-) create mode 100644 src/main/java/org/springframework/samples/petclinic/system/ExampleException.java delete mode 100644 src/test/java/nl/utwente/bpsd/selenium/FailingIT.java diff --git a/src/main/java/org/springframework/samples/petclinic/PetClinicApplication.java b/src/main/java/org/springframework/samples/petclinic/PetClinicApplication.java index 224c326c7..5265770dc 100644 --- a/src/main/java/org/springframework/samples/petclinic/PetClinicApplication.java +++ b/src/main/java/org/springframework/samples/petclinic/PetClinicApplication.java @@ -21,14 +21,14 @@ import org.springframework.boot.autoconfigure.SpringBootApplication; /** * PetClinic Spring Boot Application. - * + * * @author Dave Syer * */ @SpringBootApplication public class PetClinicApplication { - public static void main(String[] args) throws Exception { + public static void main(String[] args) { SpringApplication.run(PetClinicApplication.class, args); } diff --git a/src/main/java/org/springframework/samples/petclinic/owner/Owner.java b/src/main/java/org/springframework/samples/petclinic/owner/Owner.java index f6fcae7ac..e471b5c8a 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/Owner.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/Owner.java @@ -126,13 +126,13 @@ public class Owner extends Person { * @param name to test * @return true if pet name is already in use */ - public Pet getPet(String name, boolean ignoreNew) { - name = name.toLowerCase(); + public Pet getPet(final String name, final boolean ignoreNew) { + String lowerCaseName = name.toLowerCase(); for (Pet pet : getPetsInternal()) { if (!ignoreNew || !pet.isNew()) { String compName = pet.getName(); compName = compName.toLowerCase(); - if (compName.equals(name)) { + if (compName.equals(lowerCaseName)) { return pet; } } diff --git a/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java b/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java index ef3169b70..58be460a0 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java @@ -78,7 +78,7 @@ class OwnerController { } @RequestMapping(value = "/owners", method = RequestMethod.GET) - public String processFindForm(Owner owner, BindingResult result, Map model) { + public String processFindForm(final Owner owner, final BindingResult result, final Map model) { // allow parameterless GET request for /owners to return all records if (owner.getLastName() == null) { @@ -93,8 +93,8 @@ class OwnerController { return "owners/findOwners"; } else if (results.size() == 1) { // 1 owner found - owner = results.iterator().next(); - return "redirect:/owners/" + owner.getId(); + Owner founndOwner = results.iterator().next(); + return "redirect:/owners/" + founndOwner.getId(); } else { // multiple owners found model.put("selections", results); diff --git a/src/main/java/org/springframework/samples/petclinic/system/CrashController.java b/src/main/java/org/springframework/samples/petclinic/system/CrashController.java index a702cfc8f..5fe9a3294 100644 --- a/src/main/java/org/springframework/samples/petclinic/system/CrashController.java +++ b/src/main/java/org/springframework/samples/petclinic/system/CrashController.java @@ -31,8 +31,7 @@ class CrashController { @RequestMapping(value = "/oups", method = RequestMethod.GET) public String triggerException() { - throw new RuntimeException( - "Expected: controller used to showcase what " + "happens when an exception is thrown"); + throw new ExampleException("Expected: controller used to showcase what happens when an exception is thrown"); } } diff --git a/src/main/java/org/springframework/samples/petclinic/system/ExampleException.java b/src/main/java/org/springframework/samples/petclinic/system/ExampleException.java new file mode 100644 index 000000000..bcf5e2bdd --- /dev/null +++ b/src/main/java/org/springframework/samples/petclinic/system/ExampleException.java @@ -0,0 +1,15 @@ +package org.springframework.samples.petclinic.system; + +/** + * @author Martijn + * @since 21-6-2017. + */ +public class ExampleException extends RuntimeException { + public ExampleException(String message) { + super(ExampleException.class.getSimpleName() + ": " + message); + } + + protected ExampleException(String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace) { + super(ExampleException.class.getSimpleName() + ": " + message, cause, enableSuppression, writableStackTrace); + } +} diff --git a/src/main/java/org/springframework/samples/petclinic/vet/VetController.java b/src/main/java/org/springframework/samples/petclinic/vet/VetController.java index 8ddcca60a..7911aa702 100644 --- a/src/main/java/org/springframework/samples/petclinic/vet/VetController.java +++ b/src/main/java/org/springframework/samples/petclinic/vet/VetController.java @@ -31,11 +31,11 @@ import org.springframework.web.bind.annotation.ResponseBody; @Controller class VetController { - private final VetRepository vets; + private final VetRepository vetRepository; @Autowired public VetController(VetRepository clinicService) { - this.vets = clinicService; + this.vetRepository = clinicService; } @RequestMapping(value = { "/vets.html" }) @@ -43,7 +43,7 @@ class VetController { // Here we are returning an object of type 'Vets' rather than a collection of Vet // objects so it is simpler for Object-Xml mapping Vets vets = new Vets(); - vets.getVetList().addAll(this.vets.findAll()); + vets.getVetList().addAll(this.vetRepository.findAll()); model.put("vets", vets); return "vets/vetList"; } @@ -53,7 +53,7 @@ class VetController { // Here we are returning an object of type 'Vets' rather than a collection of Vet // objects so it is simpler for JSon/Object mapping Vets vets = new Vets(); - vets.getVetList().addAll(this.vets.findAll()); + vets.getVetList().addAll(this.vetRepository.findAll()); return vets; } diff --git a/src/main/java/org/springframework/samples/petclinic/vet/VetRepository.java b/src/main/java/org/springframework/samples/petclinic/vet/VetRepository.java index 20863ce76..15c4bf9ac 100644 --- a/src/main/java/org/springframework/samples/petclinic/vet/VetRepository.java +++ b/src/main/java/org/springframework/samples/petclinic/vet/VetRepository.java @@ -18,7 +18,6 @@ package org.springframework.samples.petclinic.vet; import java.util.Collection; import org.springframework.cache.annotation.Cacheable; -import org.springframework.dao.DataAccessException; import org.springframework.data.repository.Repository; import org.springframework.transaction.annotation.Transactional; @@ -40,7 +39,7 @@ public interface VetRepository extends Repository { */ @Transactional(readOnly = true) @Cacheable("vets") - Collection findAll() throws DataAccessException; + Collection findAll(); } diff --git a/src/main/java/org/springframework/samples/petclinic/vet/Vets.java b/src/main/java/org/springframework/samples/petclinic/vet/Vets.java index f5b24c3fc..152fa560e 100644 --- a/src/main/java/org/springframework/samples/petclinic/vet/Vets.java +++ b/src/main/java/org/springframework/samples/petclinic/vet/Vets.java @@ -30,14 +30,14 @@ import javax.xml.bind.annotation.XmlRootElement; @XmlRootElement public class Vets { - private List vets; + private List vetList; @XmlElement public List getVetList() { - if (vets == null) { - vets = new ArrayList<>(); + if (vetList == null) { + vetList = new ArrayList<>(); } - return vets; + return vetList; } } diff --git a/src/main/java/org/springframework/samples/petclinic/visit/VisitRepository.java b/src/main/java/org/springframework/samples/petclinic/visit/VisitRepository.java index c7853d170..122599886 100644 --- a/src/main/java/org/springframework/samples/petclinic/visit/VisitRepository.java +++ b/src/main/java/org/springframework/samples/petclinic/visit/VisitRepository.java @@ -17,7 +17,6 @@ package org.springframework.samples.petclinic.visit; import java.util.List; -import org.springframework.dao.DataAccessException; import org.springframework.data.repository.Repository; import org.springframework.samples.petclinic.model.BaseEntity; @@ -38,7 +37,7 @@ public interface VisitRepository extends Repository { * @param visit the Visit to save * @see BaseEntity#isNew */ - void save(Visit visit) throws DataAccessException; + void save(Visit visit); List findByPetId(Integer petId); diff --git a/src/test/java/nl/utwente/bpsd/selenium/FailingIT.java b/src/test/java/nl/utwente/bpsd/selenium/FailingIT.java deleted file mode 100644 index 87a26e3ad..000000000 --- a/src/test/java/nl/utwente/bpsd/selenium/FailingIT.java +++ /dev/null @@ -1,26 +0,0 @@ -package nl.utwente.bpsd.selenium; - -import org.junit.Assert; -import org.junit.Ignore; -import org.junit.Test; -import org.junit.experimental.categories.Category; - -import java.net.MalformedURLException; - -/** - * @author Martijn - * @since 21-6-2017. - */ -public class FailingIT extends SeleniumBaseIT { - public FailingIT() throws MalformedURLException { - super(); - } - - @Test - @Category(SeleniumBaseIT.class) - @Ignore - public void failIT() { - Assert.fail(); - } - -} From c4dd55342da6839ac2448914be3652a77337200a Mon Sep 17 00:00:00 2001 From: Marty30 Date: Sun, 25 Jun 2017 13:37:43 +0200 Subject: [PATCH 3/9] Added more logging on the tests and added a wait --- .../nl/utwente/bpsd/selenium/AddOwnerIT.java | 2 + .../nl/utwente/bpsd/selenium/AddVisitIT.java | 4 +- .../nl/utwente/bpsd/selenium/FindOwnerIT.java | 5 +- .../utwente/bpsd/selenium/SeleniumBaseIT.java | 48 +++++++++++++++++-- 4 files changed, 52 insertions(+), 7 deletions(-) diff --git a/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java b/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java index de1dfaba9..2bbba0077 100644 --- a/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java +++ b/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java @@ -41,7 +41,9 @@ public class AddOwnerIT extends SeleniumBaseIT { new Select(driver.findElement(By.name("type"))).selectByValue("hamster"); driver.findElement(By.name("name")).submit(); + waitForPageToLoad(); Assert.assertTrue(pageContainsText("Thumper")); + setTestFinished(); } } diff --git a/src/test/java/nl/utwente/bpsd/selenium/AddVisitIT.java b/src/test/java/nl/utwente/bpsd/selenium/AddVisitIT.java index c36098cda..cee199e50 100644 --- a/src/test/java/nl/utwente/bpsd/selenium/AddVisitIT.java +++ b/src/test/java/nl/utwente/bpsd/selenium/AddVisitIT.java @@ -22,7 +22,7 @@ public class AddVisitIT extends SeleniumBaseIT { @Category(SeleniumBaseIT.class) public void addOwnerTest() { driver.findElement(By.name("lastName")).submit(); - + //Go to first owner WebElement table = driver.findElement(By.tagName("table")); List cells = table.findElements(By.xpath("tr/td")); @@ -35,7 +35,9 @@ public class AddVisitIT extends SeleniumBaseIT { fillTextField(By.name("name"), "foobar"); driver.findElement(By.name("name")).submit(); + waitForPageToLoad(); Assert.assertNotNull(driver.findElement(By.xpath("//table//tr/td/dl/dd/[contains(text(), 'foobar')]"))); + setTestFinished(); } } diff --git a/src/test/java/nl/utwente/bpsd/selenium/FindOwnerIT.java b/src/test/java/nl/utwente/bpsd/selenium/FindOwnerIT.java index 17fcd7de4..57da781d9 100644 --- a/src/test/java/nl/utwente/bpsd/selenium/FindOwnerIT.java +++ b/src/test/java/nl/utwente/bpsd/selenium/FindOwnerIT.java @@ -23,6 +23,9 @@ public class FindOwnerIT extends SeleniumBaseIT { driver.get(BASE_URL+"/owners/find"); fillTextField(By.name("lastName"),"Coleman"); driver.findElement(By.name("lastName")).submit(); - Assert.assertTrue(driver.findElementsByXPath("//*[text()='Jean Coleman']").size() == 1); + waitForPageToLoad(); + Assert.assertTrue("Could not find \"Jean Coleman\" on the current page. This is the html of the current page: "+getHTML(),driver.findElements(By.xpath("//*[text()='Jean Coleman']")).size() == 1); + setTestFinished(); } + } diff --git a/src/test/java/nl/utwente/bpsd/selenium/SeleniumBaseIT.java b/src/test/java/nl/utwente/bpsd/selenium/SeleniumBaseIT.java index d4f701966..a69a41e1d 100644 --- a/src/test/java/nl/utwente/bpsd/selenium/SeleniumBaseIT.java +++ b/src/test/java/nl/utwente/bpsd/selenium/SeleniumBaseIT.java @@ -1,13 +1,17 @@ package nl.utwente.bpsd.selenium; import org.junit.After; -import org.openqa.selenium.By; -import org.openqa.selenium.chrome.ChromeDriver; +import org.openqa.selenium.*; +import org.openqa.selenium.remote.Augmenter; import org.openqa.selenium.remote.DesiredCapabilities; import org.openqa.selenium.remote.RemoteWebDriver; +import org.openqa.selenium.support.ui.ExpectedCondition; +import org.openqa.selenium.support.ui.WebDriverWait; +import java.io.File; import java.net.MalformedURLException; import java.net.URL; +import java.util.logging.Logger; /** * @@ -15,14 +19,15 @@ import java.net.URL; * @since 21-6-2017. */ public class SeleniumBaseIT { - protected final RemoteWebDriver driver; + protected final WebDriver driver; public static final String BASE_URL = "http://pet-clinic:8080/"; + private boolean testFinished = false; // public static final String BASE_URL = "http://localhost:8080/"; public SeleniumBaseIT() throws MalformedURLException { // System.setProperty("webdriver.chrome.driver","C:\\Users\\marti\\Downloads\\chromedriver_win32\\chromedriver.exe"); // this.driver = new ChromeDriver(); - this.driver = new RemoteWebDriver(new URL("http://selenium:4444/wd/hub"), DesiredCapabilities.firefox()); + this.driver = new Augmenter().augment(new RemoteWebDriver(new URL("http://selenium:4444/wd/hub"), DesiredCapabilities.firefox())); driver.get(BASE_URL); } @@ -31,13 +36,46 @@ public class SeleniumBaseIT { driver.findElement(by).sendKeys(text); } + protected void setTestFinished() { + testFinished = true; + } + @After public void afterTest() { + if (!testFinished) { + if (driver instanceof TakesScreenshot) { + File screenshot = ((TakesScreenshot) driver).getScreenshotAs(OutputType.FILE); + Logger.getLogger(this.getClass().getName()).warning(screenshot.getAbsolutePath()); + } + } driver.close(); } protected boolean pageContainsText(String text) { - return driver.findElementsByXPath("//*[text()='"+text+"']").size() == 1; + return driver.findElements(By.xpath("//*[text()='"+text+"']")).size() == 1; + } + + public String getHTML() { + return driver.findElement(By.xpath("//html")).getAttribute("innerHTML"); + } + + protected void waitForPageToLoad() { + waitFor(new PageLoadedExpectedCondition()); + } + + private void waitFor(ExpectedCondition condition) { + new WebDriverWait(driver, 3).until(condition); + } + + private class PageLoadedExpectedCondition implements ExpectedCondition { + + @Override + public Boolean apply(WebDriver webDriver) { + if (webDriver instanceof JavascriptExecutor) { + return ((JavascriptExecutor)webDriver).executeScript("return document.readyState").equals("complete"); + } + throw new ClassCastException("This webdriver is not able to execute javascript."); + } } } From cc80b7482576668a1608edc2e0a2b6bc81ec4378 Mon Sep 17 00:00:00 2001 From: Marty30 Date: Sun, 25 Jun 2017 14:03:14 +0200 Subject: [PATCH 4/9] Add an extra wait --- .../nl/utwente/bpsd/selenium/FindOwnerIT.java | 1 + .../utwente/bpsd/selenium/SeleniumBaseIT.java | 21 ++++++++++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/test/java/nl/utwente/bpsd/selenium/FindOwnerIT.java b/src/test/java/nl/utwente/bpsd/selenium/FindOwnerIT.java index 57da781d9..45050f1ab 100644 --- a/src/test/java/nl/utwente/bpsd/selenium/FindOwnerIT.java +++ b/src/test/java/nl/utwente/bpsd/selenium/FindOwnerIT.java @@ -23,6 +23,7 @@ public class FindOwnerIT extends SeleniumBaseIT { driver.get(BASE_URL+"/owners/find"); fillTextField(By.name("lastName"),"Coleman"); driver.findElement(By.name("lastName")).submit(); + waitFor(new FixedPeriod(1000)); waitForPageToLoad(); Assert.assertTrue("Could not find \"Jean Coleman\" on the current page. This is the html of the current page: "+getHTML(),driver.findElements(By.xpath("//*[text()='Jean Coleman']")).size() == 1); setTestFinished(); diff --git a/src/test/java/nl/utwente/bpsd/selenium/SeleniumBaseIT.java b/src/test/java/nl/utwente/bpsd/selenium/SeleniumBaseIT.java index a69a41e1d..e65d5a692 100644 --- a/src/test/java/nl/utwente/bpsd/selenium/SeleniumBaseIT.java +++ b/src/test/java/nl/utwente/bpsd/selenium/SeleniumBaseIT.java @@ -64,7 +64,7 @@ public class SeleniumBaseIT { waitFor(new PageLoadedExpectedCondition()); } - private void waitFor(ExpectedCondition condition) { + protected void waitFor(ExpectedCondition condition) { new WebDriverWait(driver, 3).until(condition); } @@ -78,4 +78,23 @@ public class SeleniumBaseIT { throw new ClassCastException("This webdriver is not able to execute javascript."); } } + + protected class FixedPeriod implements ExpectedCondition { + private final int time; + + public FixedPeriod(int timeInMilliseconds) { + this.time = timeInMilliseconds; + } + + @Override + public Boolean apply(WebDriver webDriver) { + try { + Thread.sleep(time); + return true; + } catch (InterruptedException e) { + e.printStackTrace(); + return false; + } + } + } } From 6a85a1ac631a44d1d4aa899e7261ef166e2c220b Mon Sep 17 00:00:00 2001 From: Marty30 Date: Sun, 25 Jun 2017 15:02:35 +0200 Subject: [PATCH 5/9] Add an extra wait and mostly fixed the addVisit test --- .../nl/utwente/bpsd/selenium/AddOwnerIT.java | 1 + .../nl/utwente/bpsd/selenium/AddVisitIT.java | 35 +++++++++++-------- .../utwente/bpsd/selenium/SeleniumBaseIT.java | 3 +- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java b/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java index 2bbba0077..37903fb44 100644 --- a/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java +++ b/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java @@ -41,6 +41,7 @@ public class AddOwnerIT extends SeleniumBaseIT { new Select(driver.findElement(By.name("type"))).selectByValue("hamster"); driver.findElement(By.name("name")).submit(); + waitFor(new FixedPeriod(1000)); waitForPageToLoad(); Assert.assertTrue(pageContainsText("Thumper")); setTestFinished(); diff --git a/src/test/java/nl/utwente/bpsd/selenium/AddVisitIT.java b/src/test/java/nl/utwente/bpsd/selenium/AddVisitIT.java index cee199e50..7a8521a35 100644 --- a/src/test/java/nl/utwente/bpsd/selenium/AddVisitIT.java +++ b/src/test/java/nl/utwente/bpsd/selenium/AddVisitIT.java @@ -10,7 +10,7 @@ import java.net.MalformedURLException; import java.util.List; /** - * @author Martijn + * @author Sophie * @since 21-6-2017. */ public class AddVisitIT extends SeleniumBaseIT { @@ -20,24 +20,29 @@ public class AddVisitIT extends SeleniumBaseIT { @Test @Category(SeleniumBaseIT.class) - public void addOwnerTest() { - driver.findElement(By.name("lastName")).submit(); + public void editPetNameAndAddAVisit() { + driver.get(BASE_URL+"/owners/find"); - //Go to first owner - WebElement table = driver.findElement(By.tagName("table")); - List cells = table.findElements(By.xpath("tr/td")); - cells.get(0).findElement(By.xpath("a")).click(); + driver.findElement(By.name("lastName")).submit(); - //Go to edit page of first pet - driver.findElement(By.xpath("//table//tr/td/table/tbody//a[text()='Edit Pet']")).click(); + //Go to first owner + WebElement table = driver.findElement(By.tagName("table")); + List cells = table.findElements(By.xpath(".//tr/td")); + cells.get(0).findElement(By.xpath("a")).click(); - //Edit Name of pet - fillTextField(By.name("name"), "foobar"); - driver.findElement(By.name("name")).submit(); + //Go to edit page of first pet + driver.findElement(By.xpath("//table//tr/td/table/tbody//a[contains(text(),'Edit')]")).click(); - waitForPageToLoad(); - Assert.assertNotNull(driver.findElement(By.xpath("//table//tr/td/dl/dd/[contains(text(), 'foobar')]"))); - setTestFinished(); + //Edit Name of pet + fillTextField(By.name("name"), "foobar"); + driver.findElement(By.name("name")).submit(); + + waitFor(new FixedPeriod(1000)); + waitForPageToLoad(); + Assert.assertNotNull(driver.findElement(By.xpath("//table//tr/td/dl/dd[contains(text(), 'foobar')]"))); + + //TODO add a visit + setTestFinished(); } } diff --git a/src/test/java/nl/utwente/bpsd/selenium/SeleniumBaseIT.java b/src/test/java/nl/utwente/bpsd/selenium/SeleniumBaseIT.java index e65d5a692..0cf386804 100644 --- a/src/test/java/nl/utwente/bpsd/selenium/SeleniumBaseIT.java +++ b/src/test/java/nl/utwente/bpsd/selenium/SeleniumBaseIT.java @@ -2,6 +2,7 @@ package nl.utwente.bpsd.selenium; import org.junit.After; import org.openqa.selenium.*; +import org.openqa.selenium.chrome.ChromeDriver; import org.openqa.selenium.remote.Augmenter; import org.openqa.selenium.remote.DesiredCapabilities; import org.openqa.selenium.remote.RemoteWebDriver; @@ -20,8 +21,8 @@ import java.util.logging.Logger; */ public class SeleniumBaseIT { protected final WebDriver driver; - public static final String BASE_URL = "http://pet-clinic:8080/"; private boolean testFinished = false; + public static final String BASE_URL = "http://pet-clinic:8080/"; // public static final String BASE_URL = "http://localhost:8080/"; public SeleniumBaseIT() throws MalformedURLException { From b544ab80bdd1c610eb1e8d3d9bb1ed4146b3d83e Mon Sep 17 00:00:00 2001 From: Marty30 Date: Sun, 25 Jun 2017 15:13:00 +0200 Subject: [PATCH 6/9] Add more waits --- src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java | 2 +- src/test/java/nl/utwente/bpsd/selenium/AddVisitIT.java | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java b/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java index 37903fb44..42f62c063 100644 --- a/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java +++ b/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java @@ -43,7 +43,7 @@ public class AddOwnerIT extends SeleniumBaseIT { waitFor(new FixedPeriod(1000)); waitForPageToLoad(); - Assert.assertTrue(pageContainsText("Thumper")); + Assert.assertTrue("Could not locate \"Thumper\" on the page. This is the html of the current page: "+getHTML(),pageContainsText("Thumper")); setTestFinished(); } diff --git a/src/test/java/nl/utwente/bpsd/selenium/AddVisitIT.java b/src/test/java/nl/utwente/bpsd/selenium/AddVisitIT.java index 7a8521a35..864ae7103 100644 --- a/src/test/java/nl/utwente/bpsd/selenium/AddVisitIT.java +++ b/src/test/java/nl/utwente/bpsd/selenium/AddVisitIT.java @@ -24,6 +24,8 @@ public class AddVisitIT extends SeleniumBaseIT { driver.get(BASE_URL+"/owners/find"); driver.findElement(By.name("lastName")).submit(); + waitFor(new FixedPeriod(1000)); + waitForPageToLoad(); //Go to first owner WebElement table = driver.findElement(By.tagName("table")); From c9d54ba6b8f3bd3aad48077a0ae895260a27a74c Mon Sep 17 00:00:00 2001 From: Marty30 Date: Sun, 25 Jun 2017 15:39:03 +0200 Subject: [PATCH 7/9] Add more waits --- src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java | 5 ++--- src/test/java/nl/utwente/bpsd/selenium/AddVisitIT.java | 3 +-- src/test/java/nl/utwente/bpsd/selenium/FindOwnerIT.java | 1 - src/test/java/nl/utwente/bpsd/selenium/SeleniumBaseIT.java | 3 ++- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java b/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java index 42f62c063..1bcaf0099 100644 --- a/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java +++ b/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java @@ -6,7 +6,6 @@ import org.junit.experimental.categories.Category; import org.openqa.selenium.By; import org.openqa.selenium.support.ui.ExpectedConditions; import org.openqa.selenium.support.ui.Select; -import org.openqa.selenium.support.ui.WebDriverWait; import java.net.MalformedURLException; @@ -31,17 +30,17 @@ public class AddOwnerIT extends SeleniumBaseIT { fillTextField(By.name("city"), "Enschede"); fillTextField(By.name("telephone"), "0534890000"); driver.findElement(By.name("telephone")).submit(); + waitForPageToLoad(); Assert.assertTrue(pageContainsText("Sophie Lathouwers")); //Add a pet - new WebDriverWait(driver, 3).until(ExpectedConditions.presenceOfAllElementsLocatedBy(By.linkText("Add New Pet"))); + waitFor(ExpectedConditions.presenceOfAllElementsLocatedBy(By.linkText("Add New Pet"))); driver.findElement(By.linkText("Add New Pet")).click(); fillTextField(By.name("name"), "Thumper"); fillTextField(By.name("birthDate"), "1942/08/09"); new Select(driver.findElement(By.name("type"))).selectByValue("hamster"); driver.findElement(By.name("name")).submit(); - waitFor(new FixedPeriod(1000)); waitForPageToLoad(); Assert.assertTrue("Could not locate \"Thumper\" on the page. This is the html of the current page: "+getHTML(),pageContainsText("Thumper")); setTestFinished(); diff --git a/src/test/java/nl/utwente/bpsd/selenium/AddVisitIT.java b/src/test/java/nl/utwente/bpsd/selenium/AddVisitIT.java index 864ae7103..82c2889a1 100644 --- a/src/test/java/nl/utwente/bpsd/selenium/AddVisitIT.java +++ b/src/test/java/nl/utwente/bpsd/selenium/AddVisitIT.java @@ -24,13 +24,13 @@ public class AddVisitIT extends SeleniumBaseIT { driver.get(BASE_URL+"/owners/find"); driver.findElement(By.name("lastName")).submit(); - waitFor(new FixedPeriod(1000)); waitForPageToLoad(); //Go to first owner WebElement table = driver.findElement(By.tagName("table")); List cells = table.findElements(By.xpath(".//tr/td")); cells.get(0).findElement(By.xpath("a")).click(); + waitForPageToLoad(); //Go to edit page of first pet driver.findElement(By.xpath("//table//tr/td/table/tbody//a[contains(text(),'Edit')]")).click(); @@ -39,7 +39,6 @@ public class AddVisitIT extends SeleniumBaseIT { fillTextField(By.name("name"), "foobar"); driver.findElement(By.name("name")).submit(); - waitFor(new FixedPeriod(1000)); waitForPageToLoad(); Assert.assertNotNull(driver.findElement(By.xpath("//table//tr/td/dl/dd[contains(text(), 'foobar')]"))); diff --git a/src/test/java/nl/utwente/bpsd/selenium/FindOwnerIT.java b/src/test/java/nl/utwente/bpsd/selenium/FindOwnerIT.java index 45050f1ab..57da781d9 100644 --- a/src/test/java/nl/utwente/bpsd/selenium/FindOwnerIT.java +++ b/src/test/java/nl/utwente/bpsd/selenium/FindOwnerIT.java @@ -23,7 +23,6 @@ public class FindOwnerIT extends SeleniumBaseIT { driver.get(BASE_URL+"/owners/find"); fillTextField(By.name("lastName"),"Coleman"); driver.findElement(By.name("lastName")).submit(); - waitFor(new FixedPeriod(1000)); waitForPageToLoad(); Assert.assertTrue("Could not find \"Jean Coleman\" on the current page. This is the html of the current page: "+getHTML(),driver.findElements(By.xpath("//*[text()='Jean Coleman']")).size() == 1); setTestFinished(); diff --git a/src/test/java/nl/utwente/bpsd/selenium/SeleniumBaseIT.java b/src/test/java/nl/utwente/bpsd/selenium/SeleniumBaseIT.java index 0cf386804..333b62d5a 100644 --- a/src/test/java/nl/utwente/bpsd/selenium/SeleniumBaseIT.java +++ b/src/test/java/nl/utwente/bpsd/selenium/SeleniumBaseIT.java @@ -62,10 +62,11 @@ public class SeleniumBaseIT { } protected void waitForPageToLoad() { + waitFor(new FixedPeriod(333)); waitFor(new PageLoadedExpectedCondition()); } - protected void waitFor(ExpectedCondition condition) { + protected void waitFor(ExpectedCondition condition) { new WebDriverWait(driver, 3).until(condition); } From a8c71524acc2887593b437bd616730d4ffd10b7b Mon Sep 17 00:00:00 2001 From: Marty30 Date: Sun, 25 Jun 2017 15:50:59 +0200 Subject: [PATCH 8/9] More logging --- src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java b/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java index 1bcaf0099..91158ea62 100644 --- a/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java +++ b/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java @@ -31,7 +31,7 @@ public class AddOwnerIT extends SeleniumBaseIT { fillTextField(By.name("telephone"), "0534890000"); driver.findElement(By.name("telephone")).submit(); waitForPageToLoad(); - Assert.assertTrue(pageContainsText("Sophie Lathouwers")); + Assert.assertTrue("Could not locate \"Sophie Lathouwers\" on the page. This is the html of the current page: "+getHTML(), pageContainsText("Sophie Lathouwers")); //Add a pet waitFor(ExpectedConditions.presenceOfAllElementsLocatedBy(By.linkText("Add New Pet"))); From 71b820ac2d6ac69809916c837c27614193651262 Mon Sep 17 00:00:00 2001 From: Marty30 Date: Sun, 25 Jun 2017 16:48:03 +0200 Subject: [PATCH 9/9] different wait to try --- src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java b/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java index 91158ea62..52a98b4f3 100644 --- a/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java +++ b/src/test/java/nl/utwente/bpsd/selenium/AddOwnerIT.java @@ -31,6 +31,7 @@ public class AddOwnerIT extends SeleniumBaseIT { fillTextField(By.name("telephone"), "0534890000"); driver.findElement(By.name("telephone")).submit(); waitForPageToLoad(); + waitFor(ExpectedConditions.presenceOfElementLocated(By.xpath("//*[text()='Sophie Lathouwers']"))); Assert.assertTrue("Could not locate \"Sophie Lathouwers\" on the page. This is the html of the current page: "+getHTML(), pageContainsText("Sophie Lathouwers")); //Add a pet