Selenium tests #212

Starzu · 2018-07-30T11:31:27Z

Copy-paste from guide + configuration

Starzu · 2018-07-30T13:35:51Z

The tests seem to be flaky on travis. I want to investigate it before merge.

Starzu · 2018-07-31T10:12:02Z

It seems that Selenium with Chrome miss the button clicks sometimes. Firefox is much more stable.

ddworak

Left some comments to the build setup itself and found you the best reviewer for the rest, good luck! 🤣

ddworak · 2018-08-09T11:34:14Z

.travis.yml

+addons:
+  firefox: latest
+  chrome: stable
+
 before_script:
  - "export DISPLAY=:99.0"


I think it might be possible to simplify this with xvfb-run wrapper: https://docs.travis-ci.com/user/gui-and-headless-browsers/#using-the-xvfb-run-wrapper

ddworak · 2018-08-09T11:35:23Z

.travis.yml

-  - wget http://chromedriver.storage.googleapis.com/2.36/chromedriver_linux64.zip
-  - unzip chromedriver_linux64.zip -d selenium-bin
-  - export PATH=$PWD/selenium-bin:$PATH
+  # Instal google-chrome-driver


With dist: trusty it should be possible to just:

before_install: - sudo apt-get update - sudo apt-get install chromium-chromedriver

ddworak · 2018-08-09T11:37:36Z

build.sbt

@@ -350,3 +350,110 @@ lazy val `benchmarks-frontend` = project.in(file("benchmarks/frontend"))
    libraryDependencies ++= Dependencies.benchmarksFrontendDeps.value,
    Compile / scalaJSUseMainModuleInitializer := true,
  )
+
+lazy val `selenium-shared` = project.in(file("selenium/shared"))


I'd much rather we setup the selenium project in a separate build.sbt file, no need to make this one more complex.

g-skiba

Some questions and suggestions, pls let me know what you think

g-skiba · 2018-08-16T07:39:33Z

selenium/shared/src/main/scala/io/udash/selenium/rpc/demos/DemosServerRPC.scala

+  def translations(): RemoteTranslationRPC
+  def exceptions(): ExceptionsRPC
+
+  def call(): CallServerRPC


This method is unused, I think you're missing some demo.

g-skiba · 2018-08-16T08:16:43Z

selenium/frontend/src/main/scala/io/udash/selenium/rpc/PingClient.scala

+    pongListeners.foreach(l => l(id))
+  }
+
+  def registerPongListener(listener: Int => Any) = pongListeners += listener


I don't think you want to expose the pongListeners in these methods but rather make them return Unit

g-skiba · 2018-08-16T09:46:00Z

selenium/frontend/src/main/scala/io/udash/selenium/views/IntroView.scala

+      li(a(href := FrontendRoutingDemosState(None).url)("Frontend Routing")),
+      li(a(href := RpcDemosState.url)("RPC")),
+      li(a(href := RestDemosState.url)("REST")),
+      li(a(href := I18nDemosState.url)("i18n"))


You might be missing two links: to /activity which I think is the missing demo and to /jquery

g-skiba · 2018-08-16T09:57:11Z

selenium/frontend/src/main/scala/io/udash/selenium/views/demos/UrlLoggingDemo.scala

+
+import scala.collection.mutable.ListBuffer
+
+object UrlLoggingDemo {


This demo does not seem to be effectively used (presented) anywhere, is it intentional?

g-skiba · 2018-08-16T10:38:26Z

.../frontend/src/main/scala/io/udash/selenium/views/demos/frontend/TextInputDemoComponent.scala

+    div(BootstrapStyles.Grid.colMd4)(
+      UdashInputGroup()(
+        UdashInputGroup.input(
+          NumberInput(age.transform(_.toString, Integer.parseInt))(maxlength := "6").render


The maxlength attribute does not work this way for number input. min and max attributes might be used instead.

Copy-paste things...

g-skiba · 2018-08-16T10:55:44Z

selenium/backend/src/test/scala/io/udash/selenium/frontend/FrontendBindingsTest.scala

+        }
+      }
+
+      for (_ <- 1 to 3) {


This takes at least 24s (3 iterations * 4 names * 2s interval between changes) - are we OK with this?

We had no choice in case of human-friendly demos in the guide. Now we can reduce these intervals.

g-skiba · 2018-08-16T12:01:46Z

selenium/backend/src/test/scala/io/udash/selenium/SeleniumTest.scala

+
+  override protected def afterAll(): Unit = {
+    server.stop()
+    driver.close()


I think driver.quit() should be called instead (or maybe additionally?) in order to get rid of the driver process although from what I know it does not always work.

g-skiba · 2018-08-16T12:44:22Z

selenium/backend/src/test/scala/io/udash/selenium/frontend/FrontendFormsTest.scala

+        val checkbox = checkboxes.findElement(new ByClassName(s"checkbox-demo-$propertyName"))
+        checkbox.click()
+        eventually {
+          checkboxes.findElements(new ByCssSelector(s"[data-bind=$propertyName]")).asScala.forall(el => {


You might want to consider replacing forall with foreach with assertion inside so that you get any more information than just that the you got false instead of true

g-skiba · 2018-08-16T12:59:45Z

selenium/backend/src/test/scala/io/udash/selenium/frontend/FrontendRoutingTest.scala

+        linkChanger.clear()
+        linkChanger.sendKeys(s)
+        eventually {
+          val escaped = s"/frontend/routing/${s.replaceAll(" ", "%20").replaceAll("#", "%23")}"


You could use URLEncoder

ddworak · 2018-09-25T08:17:51Z

rest/backend/src/test/scala/io/udash/rest/server/EndpointsIntegrationTest.scala


      val eventualResponse2 =
        inexistentServerConnector.send("/non/existing/path", RESTConnector.HttpMethod.POST, Map.empty, Map.empty, "lol")
-      intercept[ConnectException](await(eventualResponse2))
+      intercept[ConnectException](Await.result(eventualResponse2, 10 seconds))


We can use a global (or even scaled) implicit timeout with the ScalaFutures syntax here:

eventualResponse2.failed.futureValue shouldBe a [ConnectException]

I don't want the global timeout, in this test I want to use this specific value.

You can introduce an implicit in the local scope. This specific value occurs at least twice here ;)

g-skiba · 2018-09-24T10:40:38Z

selenium/backend/src/test/scala/io/udash/selenium/frontend/FrontendFormsTest.scala

+          checkButtons.findElements(new ByClassName("check-buttons-demo-fruits")).asScala
+            .foreach { el =>
+              val contains = el.getText.contains(propertyName)
+              assert(if (checkbox.getAttribute("selected") != null) contains else !contains)


Here and in 85-86 I would change it to assert(checkbox.getAttribute("selected") != null) == el.getText.contains(propertyName)). But also (this applies to the whole file) using assertResult or should be assertions instead of assert together with == would still give you more information in case of test failure.

g-skiba · 2018-09-25T09:42:09Z

selenium/backend/src/test/scala/io/udash/selenium/SeleniumTest.scala

+    PatienceConfig(scaled(Span(10, Seconds)), scaled(Span(5, Millis)))
+
+//  val driver: RemoteWebDriver = new ChromeDriver(new ChromeOptions().setHeadless(false))
+  val driver: RemoteWebDriver = new FirefoxDriver(new FirefoxOptions().setHeadless(true))


The driver and the server should be started in beforeAll() because if the constructor fails, afterAll() can't be called. So for example if the server fails to start (e.g. some port is already used) then the browser and driver are not closed.

Selenium tests

c3a50fc

Starzu added this to the 0.8.0 milestone Jul 30, 2018

Starzu requested a review from ddworak July 30, 2018 11:31

Mateusz Starzec added 2 commits July 30, 2018 14:01

Scala 2.11 compilation fix

a6b4448

Chrome driver version

0a017d0

Starzu changed the title ~~Selenium tests~~ WIP: Selenium tests Jul 30, 2018

Starzu force-pushed the selenium branch 3 times, most recently from e3d0e2c to bebcc84 Compare July 31, 2018 09:00

Chrome -> Firefox + cleaner travis config

6073e8b

Starzu force-pushed the selenium branch from bebcc84 to 6073e8b Compare July 31, 2018 09:03

Starzu changed the title ~~WIP: Selenium tests~~ Selenium tests Jul 31, 2018

ddworak requested a review from g-skiba August 9, 2018 11:53

ddworak requested changes Aug 9, 2018

View reviewed changes

g-skiba suggested changes Aug 16, 2018

View reviewed changes

Minor travis config cleanup

1a4ec30

Starzu force-pushed the selenium branch from 8c2106a to 1a4ec30 Compare September 20, 2018 06:04

Mateusz Starzec and others added 11 commits September 20, 2018 08:25

RPC logging selenium test

89b1013

Code cleanup and demo timeouts reduce

6f2df6e

Routing log demo

f3e8153

Better intervals and patience config

2d5c08a

Merge branch 'master' into selenium

b2b7646

Compilation fixes

9368e99

JVM URLEncoder improvements

8d395d8

Sttp upgrade

a73d846

URLEncoder - consistent implementation for JVM and JS

d09f60d

Longer await for connection refused

c3a8c28

JVM decoder - fix for a + sign

c211290

Mateusz Starzec added 2 commits September 25, 2018 10:07

Headless firefox and higher timeouts for Travis

d55805d

Local bootstrap CSS files

8d5a708

ddworak reviewed Sep 25, 2018

View reviewed changes

g-skiba reviewed Sep 25, 2018

View reviewed changes

Improved driver and server lifecycle + minor tests improvements

5d11f28

ddworak approved these changes Sep 25, 2018

View reviewed changes

g-skiba approved these changes Sep 25, 2018

View reviewed changes

Starzu merged commit ba12af7 into master Sep 25, 2018

Starzu deleted the selenium branch September 25, 2018 11:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Selenium tests #212

Selenium tests #212

Starzu commented Jul 30, 2018

Starzu commented Jul 30, 2018

Starzu commented Jul 31, 2018

ddworak left a comment

ddworak Aug 9, 2018

ddworak Aug 9, 2018

ddworak Aug 9, 2018

g-skiba left a comment

g-skiba Aug 16, 2018

g-skiba Aug 16, 2018

g-skiba Aug 16, 2018

g-skiba Aug 16, 2018

g-skiba Aug 16, 2018

Starzu Sep 20, 2018

g-skiba Aug 16, 2018

Starzu Sep 20, 2018

g-skiba Aug 16, 2018

g-skiba Aug 16, 2018

g-skiba Aug 16, 2018

ddworak Sep 25, 2018

Starzu Sep 25, 2018

ddworak Sep 25, 2018

g-skiba Sep 24, 2018

g-skiba Sep 25, 2018


		import scala.collection.mutable.ListBuffer

		object UrlLoggingDemo {

Selenium tests #212

Selenium tests #212

Conversation

Starzu commented Jul 30, 2018

Starzu commented Jul 30, 2018

Starzu commented Jul 31, 2018

ddworak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

g-skiba left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment