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

FTP testing stability #424

Merged
merged 1 commit into from
Jul 31, 2017
Merged

FTP testing stability #424

merged 1 commit into from
Jul 31, 2017

Conversation

juanjovazquez
Copy link

@juanjovazquez juanjovazquez commented Jul 25, 2017

This PR improves testing stability on FTP connector. The key change has been to add a new jvm configuration parameter that enables a less strict random generator device on Travis Linux VMs. The previous one would hang the main process if the machine is not able to generate enough entropy. Aditionally, some test dependencies have been updated.

/cc @raboof

#422

@juanjovazquez juanjovazquez force-pushed the 422-testing-stability branch from 0c12fe8 to f40609a Compare July 27, 2017 17:16
@juanjovazquez juanjovazquez changed the title WIP: FTP testing stability FTP testing stability Jul 27, 2017
@juanjovazquez juanjovazquez reopened this Jul 27, 2017
@juanjovazquez
Copy link
Author

A mqtt test is not working.

@raboof
Copy link
Contributor

raboof commented Jul 28, 2017

Unfortunately Travis doesn't keep results for restarted builds, but that could probably be #290 ? Going forward #419 might reduce tests for one subproject interfering with another (though it wouldn't have helped in this case as you updated the build configuration).

.jvmopts Outdated
@@ -1,4 +1,5 @@
-Dfile.encoding=UTF8
-Djava.security.egd=file:/dev/./urandom
Copy link
Contributor

Choose a reason for hiding this comment

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

Good find, too bad this isn't the default as AFAIK this is not any less secure on modern systems (https://www.2uo.de/myths-about-urandom/)

Copy link
Member

Choose a reason for hiding this comment

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

Except for directly after booting identical virtual machines (in the end of that article).

Copy link
Author

Choose a reason for hiding this comment

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

@raboof Very interesting article. And, yes, this was a tough work indeed. 4 hours of hard debug session for just a new line on a configuration file. This job is ridicule sometimes...

Copy link
Contributor

Choose a reason for hiding this comment

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

Looking a bit closer I'm not sure this will have the intended effect: it will add -Djava.security.egd=file:/dev/./urandom to the JVM running sbt-launch.jar, but tests run in a forked JVM and it looks like this parameter is not passed on to the forked JVM. Perhaps we should configure this parameter somewhere in sbt instead to make sure it is passed to the forked JVM? Also it might be good to mention this issue in the documentation for this module.

Copy link
Author

Choose a reason for hiding this comment

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

@raboof have you checked that this parameter is not effectively inherited in the forked process?

Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't look like it, when running ps while executing the tests in the ftp subproject:

$ ps aux | grep java
aengelen 15743  148  5.8 5670344 955312 pts/13 Sl+  13:42   1:35 java -Dfile.encoding=UTF8 -Djava.security.egd=file:/dev/./urandom -Xms1g -Xmx1g -Xss6M -XX
:+CMSClassUnloadingEnabled -XX:+UseConcMarkSweepGC -Djna.nosys=true -jar /usr/share/sbt/bin/sbt-launch.jar
aengelen 16297  133  3.5 9874176 584380 pts/13 Sl+  13:43   0:47 /usr/lib/jvm/java-8-openjdk-amd64/jre/bin/java -classpath /home/aengelen/dev/alpakka/ftp/t
arget/scala-2.12/test-classes:/home/aengelen/dev/alpakka/ftp/target/scala-2.12/classes:/home/aengelen/.ivy2/cache/org.scala-lang/scala-library/jars/scala-l
ibrary-2.12.2.jar:/home/aengelen/.ivy2/cache/com.typesafe.akka/akka-stream_2.12/jars/akka-stream_2.12-2.4.19.jar:/home/aengelen/.ivy2/cache/com.typesafe.ak
ka/akka-actor_2.12/jars/akka-actor_2.12-2.4.19.jar:/home/aengelen/.ivy2/cache/com.typesafe/config/bundles/config-1.3.0.jar:/home/aengelen/.ivy2/cache/org.s
cala-lang.modules/scala-java8-compat_2.12/bundles/scala-java8-compat_2.12-0.8.0.jar:/home/aengelen/.ivy2/cache/org.reactivestreams/reactive-streams/jars/re
active-streams-1.0.0.jar:/home/aengelen/.ivy2/cache/com.typesafe/ssl-config-core_2.12/bundles/ssl-config-core_2.12-0.2.1.jar:/home/aengelen/.ivy2/cache/org
.scala-lang.modules/scala-parser-combinators_2.12/bundles/scala-parser-combinators_2.12-1.0.4.jar:/home/aengelen/.ivy2/cache/commons-net/commons-net/jars/c
ommons-net-3.6.jar:/home/aengelen/.ivy2/cache/com.hierynomus/sshj/jars/sshj-0.21.1.jar:/home/aengelen/.ivy2/cache/org.bouncycastle/bcprov-jdk15on/jars/bcpr
ov-jdk15on-1.56.jar:/home/aengelen/.ivy2/cache/org.bouncycastle/bcpkix-jdk15on/jars/bcpkix-jdk15on-1.56.jar:/home/aengelen/.ivy2/cache/com.jcraft/jzlib/jar
s/jzlib-1.1.3.jar:/home/aengelen/.ivy2/cache/net.i2p.crypto/eddsa/bundles/eddsa-0.2.0.jar:/home/aengelen/.ivy2/cache/com.typesafe.akka/akka-stream-testkit_
2.12/jars/akka-stream-testkit_2.12-2.4.19.jar:/home/aengelen/.ivy2/cache/com.typesafe.akka/akka-testkit_2.12/jars/akka-testkit_2.12-2.4.19.jar:/home/aengel
en/.ivy2/cache/org.scalatest/scalatest_2.12/bundles/scalatest_2.12-3.0.1.jar:/home/aengelen/.ivy2/cache/org.scalactic/scalactic_2.12/bundles/scalactic_2.12
-3.0.1.jar:/home/aengelen/.ivy2/cache/org.scala-lang/scala-reflect/jars/scala-reflect-2.12.2.jar:/home/aengelen/.ivy2/cache/org.scala-lang.modules/scala-xm
l_2.12/bundles/scala-xml_2.12-1.0.5.jar:/home/aengelen/.ivy2/cache/com.novocode/junit-interface/jars/junit-interface-0.11.jar:/home/aengelen/.ivy2/cache/ju
nit/junit/jars/junit-4.12.jar:/home/aengelen/.ivy2/cache/org.hamcrest/hamcrest-core/jars/hamcrest-core-1.3.jar:/home/aengelen/.ivy2/cache/org.scala-sbt/tes
t-interface/jars/test-interface-1.0.jar:/home/aengelen/.ivy2/cache/org.apache.ftpserver/ftpserver-core/bundles/ftpserver-core-1.1.1.jar:/home/aengelen/.ivy
2/cache/org.apache.ftpserver/ftplet-api/bundles/ftplet-api-1.1.1.jar:/home/aengelen/.ivy2/cache/org.apache.mina/mina-core/bundles/mina-core-2.0.16.jar:/hom
e/aengelen/.ivy2/cache/org.apache.sshd/sshd-core/jars/sshd-core-1.6.0.jar:/home/aengelen/.ivy2/cache/org.slf4j/slf4j-api/jars/slf4j-api-1.7.25.jar:/home/ae
ngelen/.ivy2/cache/com.google.jimfs/jimfs/bundles/jimfs-1.1.jar:/home/aengelen/.ivy2/cache/com.google.guava/guava/bundles/guava-18.0.jar:/home/aengelen/.iv
y2/cache/ch.qos.logback/logback-classic/jars/logback-classic-1.1.7.jar:/home/aengelen/.ivy2/cache/ch.qos.logback/logback-core/jars/logback-core-1.1.7.jar:/
home/aengelen/.sbt/boot/scala-2.10.6/org.scala-sbt/sbt/0.13.15/test-agent-0.13.15.jar:/home/aengelen/.sbt/boot/scala-2.10.6/org.scala-sbt/sbt/0.13.15/test-
interface-1.0.jar sbt.ForkMain 37621

Copy link
Author

Choose a reason for hiding this comment

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

Ok, I see... I'll check it ASAP. Thanks!.

Copy link
Author

Choose a reason for hiding this comment

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

@raboof please review if it's what you had in mind. Thanks.

@juanjovazquez
Copy link
Author

Definitely #419 will improve the experience. Currently it's really annoying stumbling across errors that are not related to the component you're working on.

.jvmopts Outdated
@@ -1,4 +1,5 @@
-Dfile.encoding=UTF8
-Djava.security.egd=file:/dev/./urandom
Copy link
Contributor

Choose a reason for hiding this comment

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

Looking a bit closer I'm not sure this will have the intended effect: it will add -Djava.security.egd=file:/dev/./urandom to the JVM running sbt-launch.jar, but tests run in a forked JVM and it looks like this parameter is not passed on to the forked JVM. Perhaps we should configure this parameter somewhere in sbt instead to make sure it is passed to the forked JVM? Also it might be good to mention this issue in the documentation for this module.

@juanjovazquez juanjovazquez force-pushed the 422-testing-stability branch from f40609a to 57e5e0c Compare July 31, 2017 14:22
Copy link
Contributor

@raboof raboof left a comment

Choose a reason for hiding this comment

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

Just tested it and this seems to indeed make a nice difference!

@raboof raboof merged commit f6131b0 into akka:master Jul 31, 2017
@juanjovazquez juanjovazquez deleted the 422-testing-stability branch August 23, 2017 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants