vaadin / board

Framework 8 version of vaadin-board
Other
8 stars 10 forks source link

Initial Tests setup #36

Closed rogozinds closed 7 years ago

rogozinds commented 7 years ago

This change is Reviewable

alvarezguille commented 7 years ago

Review status: 0 of 18 files reviewed at latest revision, 4 unresolved discussions, some commit checks broke.


a discussion (no related file): isn't maven-failsafe-plugin config missing? should the the test classes end in IT.java or any of the default include pattern?


integration-tests/src/main/java/com/vaadin/addon/board/TestServlet.java, line 9 at r2 (raw file):

//@WebServlet(urlPatterns = "/*", name = "TestServlet", asyncSupported = true, initParams = { // @WebInitParam(name = "UIProvider", value = "com.vaadin.addon.board.testUI.TestUIProvider") //}) //@VaadinServletConfiguration(ui = ExampleUI.class, productionMode = false, widgetset ="com.vaadin.addon.board.AppWidgetSet")

remove commented code


integration-tests/src/main/java/com/vaadin/addon/board/testUI/BasicUI.java, line 9 at r2 (raw file):

@Theme("mytheme")

can this be set up for all tests UIs at once?


integration-tests/src/test/java/com/vaadin/addon/board/testbenchtests/AbstractParallelTest.java, line 128 at r2 (raw file):

Quoted 9 lines of code… > //TODO this should be removed when component itself removes the license checker. > private void removeLicenseChecker(){ > WebDriver driver = getDriver(); > JavascriptExecutor js; > if (driver instanceof JavascriptExecutor) { > js = (JavascriptExecutor) driver; > js.executeScript("var elem = document.querySelector(\"vaadin-license-dialog\");" > + "if(elem) {elem.style=\"display:none\";}"); > }

  }

can you mention this in DASH-46 so it's not forgotten?


Comments from Reviewable

rogozinds commented 7 years ago

Review status: 0 of 19 files reviewed at latest revision, 4 unresolved discussions.


integration-tests/src/main/java/com/vaadin/addon/board/TestServlet.java, line 9 at r2 (raw file):

Previously, alvarezguille (Guille) wrote…
> //@WebServlet(urlPatterns = "/*", name = "TestServlet", asyncSupported = true, initParams = { > // @WebInitParam(name = "UIProvider", value = "com.vaadin.addon.board.testUI.TestUIProvider") > //}) > //@VaadinServletConfiguration(ui = ExampleUI.class, productionMode = false, widgetset ="com.vaadin.addon.board.AppWidgetSet") remove commented code

Done.


integration-tests/src/main/java/com/vaadin/addon/board/testUI/BasicUI.java, line 9 at r2 (raw file):

Previously, alvarezguille (Guille) wrote…
> @Theme("mytheme") can this be set up for all tests UIs at once?

Done.


integration-tests/src/test/java/com/vaadin/addon/board/testbenchtests/AbstractParallelTest.java, line 128 at r2 (raw file):

Previously, alvarezguille (Guille) wrote…
> //TODO this should be removed when component itself removes the license checker. > private void removeLicenseChecker(){ > WebDriver driver = getDriver(); > JavascriptExecutor js; > if (driver instanceof JavascriptExecutor) { > js = (JavascriptExecutor) driver; > js.executeScript("var elem = document.querySelector(\"vaadin-license-dialog\");" > + "if(elem) {elem.style=\"display:none\";}"); > } >   > } can you mention this in DASH-46 so it's not forgotten?

Done.


Comments from Reviewable

alvarezguille commented 7 years ago

Reviewed 9 of 21 files at r2, 15 of 15 files at r3. Review status: 20 of 22 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

alvarezguille commented 7 years ago

Reviewed 2 of 2 files at r4. Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable