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

backend unit tests for nml parser and writer #2829

Merged
merged 27 commits into from
Jul 9, 2018
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
338be99
[WIP] backend unit test for nml parser
Jun 21, 2018
62647dc
[WIP] Unit Tests for Backend
Jun 21, 2018
6e9c331
[WIP] backend unit test now work for serializing and parsing #2826
Jun 28, 2018
bbfe6e8
nml tests now work similar to the frontend tests
Jun 28, 2018
13bca13
fix imports
Jun 28, 2018
43c54a3
Merge branch 'master' of github.com:scalableminds/webknossos into bac…
Jun 29, 2018
0108076
mark test as working to enable ci build #2829
Jun 29, 2018
6d6b241
fix compiler errors and add some tests #2829
Jun 29, 2018
865d1f4
enhance the backend tree validator with some functions the frontend a…
Jun 29, 2018
f1d318e
add docker-compose service backend-tests
jstriebel Jun 29, 2018
ffba0bc
CI: add backend tests
jstriebel Jun 29, 2018
316324f
e2e tests: move invariant config to End2EndSpec.scala
jstriebel Jun 29, 2018
55e2fa8
implement pr advice #2829
Jul 3, 2018
c4fba6c
implement some of the pr advice #2832
Jul 3, 2018
950aa53
implement pr feedback #2832
Jul 5, 2018
c8c32f8
Merge branch 'backend-unit-tests' of github.com:scalableminds/webknos…
Jul 5, 2018
8b2bc6f
implement pr advice #2829
Jul 5, 2018
aaadeaf
Merge branch 'master' into backend-unit-tests
youri-k Jul 5, 2018
33e4a54
Merge branch 'backend-unit-tests' into backend-nml-parser-improve-val…
youri-k Jul 5, 2018
70b5c56
fix merge errors and make code DRYer #2832
Jul 5, 2018
fc42f40
Merge branch 'master' into backend-unit-tests
jstriebel Jul 5, 2018
206145b
Merge branch 'master' into backend-unit-tests
youri-k Jul 9, 2018
4dadc6e
Merge pull request #2832 from scalableminds/backend-nml-parser-improv…
youri-k Jul 9, 2018
0707360
Merge branch 'master' into backend-unit-tests
youri-k Jul 9, 2018
ad3a2e8
Update CHANGELOG.md
youri-k Jul 9, 2018
e8377a2
Merge branch 'master' into backend-unit-tests
youri-k Jul 9, 2018
d938fa3
Merge branch 'master' into backend-unit-tests
youri-k Jul 9, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ jobs:
- run:
name: Run frontend tests
command: docker-compose run base yarn test-verbose
- run:
name: Run backend tests
command: docker-compose run backend-tests
- run:
name: Run end-to-end tests
command: |
Expand Down
14 changes: 11 additions & 3 deletions app/models/annotation/nml/NmlParser.scala
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ object NmlParser extends LazyLogging with ProtoGeometryImplicits {
}

private def parseEditRotation(node: NodeSeq) = {
node.headOption.flatMap(parseRotation)
node.headOption.flatMap(parseRotationForParams)
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this fixes a bug you found with the tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it is because of difference in parsing EditRotation of nodes and the general param

}

private def parseZoomLevel(node: NodeSeq) = {
Expand All @@ -166,7 +166,15 @@ object NmlParser extends LazyLogging with ProtoGeometryImplicits {
} yield Point3D(x, y, z)
}

private def parseRotation(node: NodeSeq) = {
private def parseRotationForParams(node: XMLNode) = {
for {
rotX <- (node \ "@xRot").text.toDoubleOpt
rotY <- (node \ "@yRot").text.toDoubleOpt
rotZ <- (node \ "@zRot").text.toDoubleOpt
} yield Vector3D(rotX, rotY, rotZ)
}

private def parseRotationForNode(node: XMLNode) = {
for {
rotX <- (node \ "@rotX").text.toDoubleOpt
rotY <- (node \ "@rotY").text.toDoubleOpt
Expand Down Expand Up @@ -282,7 +290,7 @@ object NmlParser extends LazyLogging with ProtoGeometryImplicits {
val timestamp = parseTimestamp(node)
val bitDepth = parseBitDepth(node)
val interpolation = parseInterpolation(node)
val rotation = parseRotation(node).getOrElse(NodeDefaults.rotation)
val rotation = parseRotationForNode(node).getOrElse(NodeDefaults.rotation)
Node(id, position, rotation, radius, viewport, resolution, bitDepth, interpolation, timestamp)
}
}
Expand Down
19 changes: 10 additions & 9 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -91,30 +91,31 @@ services:
-Dpostgres.url=$${POSTGRES_URL}"
stdin_open: true

e2e-tests:
backend-tests:
extends:
service: base
ports:
- "5005:5005"
- "9000:9000"
command: sbt -v -d "test-only backend.*"

e2e-tests:
extends:
service: backend-tests
links:
- postgres
- fossildb
environment:
- POSTGRES_URL=jdbc:postgresql://postgres/webknossos_testing
command:
- bash
- -c
- >
sbt
-v -d
"test-only * --
-Dconfig.file=./conf/application.conf
-Djava.net.preferIPv4Stack=true
-Dhttp.address=0.0.0.0
"test-only e2e.* --
-Ddatastore.fossildb.address=fossildb
-Dpostgres.url=$${POSTGRES_URL}
-Dapplication.insertInitialData=false"
environment:
- POSTGRES_URL=jdbc:postgresql://postgres/webknossos_testing
-Dpostgres.url=$${POSTGRES_URL}"

postgres:
image: postgres:10-alpine
Expand Down
2 changes: 2 additions & 0 deletions project/Dependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ object Dependencies {
val reactivePlay = "org.reactivemongo" %% "play2-reactivemongo" % reactivePlayVersion
val scalaAsync = "org.scala-lang.modules" %% "scala-async" % "0.9.2"
val scalaLogging = "com.typesafe.scala-logging" %% "scala-logging" % "3.4.0"
val scalaTest = "org.scalatest" %% "scalatest" % "3.0.5" % "test"
val silhouette = "com.mohiva" %% "play-silhouette" % "3.0.5"
val silhouetteTestkit = "com.mohiva" %% "play-silhouette-testkit" % "3.0.5" % "test"
val urlHelper = "com.netaporter" %% "scala-uri" % "0.4.14"
Expand Down Expand Up @@ -86,6 +87,7 @@ object Dependencies {
ceedubs,
commonsCodec,
scalaAsync,
scalaTest,
silhouette,
silhouetteTestkit,
specs2 % Test,
Expand Down
144 changes: 144 additions & 0 deletions test/backend/NMLUnitTestSuite.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
package backend

import java.io.ByteArrayInputStream

import com.scalableminds.webknossos.datastore.SkeletonTracing._
import com.scalableminds.webknossos.datastore.VolumeTracing.VolumeTracing
import com.scalableminds.webknossos.datastore.geometry.{Point3D, Vector3D}
import com.typesafe.scalalogging.LazyLogging
import models.annotation.nml.NmlParser
import net.liftweb.common.{Box, Full}
import org.scalatest.FlatSpec
import play.api.libs.iteratee.Iteratee
import reactivemongo.bson.BSONObjectID
import utils.ObjectId

import scala.concurrent.Await
import scala.concurrent.duration.Duration


class NMLUnitTestSuite extends FlatSpec with LazyLogging {
logger.debug(s"test run")
val timestamp = 123456789

def getObjectId = ObjectId.fromBsonId(BSONObjectID.generate)

def getParsedTracing(tracing: SkeletonTracing) = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rather rename this to something more active, as it is not a typical getter. Maybe writeAndParseTracing.

val nmlEnumarator = NMLWriterTestStub.toNmlStream(tracing, None)
val arrayFuture = Iteratee.flatten(nmlEnumarator |>> Iteratee.consume[Array[Byte]]()).run
val array = Await.result(arrayFuture, Duration.Inf)
NmlParser.parse("", new ByteArrayInputStream(array))
}

def isParseSuccessful(parsedTracing: Box[(Either[SkeletonTracing, (VolumeTracing, String)], String)]): Boolean = {
parsedTracing match {
case Full(either) => either match {
case (Left(tracing), _) => {
return true
}
case _ => return false
}
case _=> return false
}
}

def createDummyNode(id: Int) = Node(id, Point3D(id, id, id), Vector3D(id, id, id), id, 1, 10, 8, id % 2 == 0, timestamp)

var tree1 = Tree(1, Seq(createDummyNode(0), createDummyNode(1), createDummyNode(2), createDummyNode(7)),
Seq(Edge(0, 1), Edge(2, 1), Edge(1, 7)), Some(Color(23, 23, 23, 1)), Seq(BranchPoint(1, 0), BranchPoint(7, 0)), Seq(Comment(0, "comment")),
"TestTree-0", timestamp, None)

var tree2 = Tree(2, Seq(createDummyNode(4), createDummyNode(5), createDummyNode(6)),
Seq(Edge(4, 5), Edge(5, 6)), Some(Color(30, 30, 30, 1)), Seq[BranchPoint](), Seq[Comment](),
"TestTree-1", timestamp, Some(1))

var treeGroup1 = TreeGroup("Axon 1", 1, Seq(TreeGroup("Blah", 3), TreeGroup("Blah 2", 4)))
var treeGroup2 = TreeGroup("Axon 2", 2)

var dummyTracing = SkeletonTracing("dummy_dataset", Seq(tree1, tree2), timestamp, None, Some(1), Point3D(1, 1, 1), Vector3D(1.0, 1.0, 1.0), 1.0, 0, None, Seq(treeGroup1, treeGroup2))

"NML writing and parsing" should "yield the same state" in {
getParsedTracing(dummyTracing) match {
case Full(either) => either match {
case (Left(tracing), _) => {
assert(tracing == dummyTracing)
}
case _ => throw new Exception
}
case _ => throw new Exception
}
}

"NML Parser" should "throw an error for invalid comment state" in {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add why this is invalid here.

val wrongTree = dummyTracing.trees(1).copy(comments = Seq(Comment(99, "test")))
val newTracing = dummyTracing.copy(trees = Seq(dummyTracing.trees(0), wrongTree))

assert(true)//!isParseSuccessful(getParsedTracing(newTracing)))
//TODO: The parser currently doesn't check this
}

it should "throw an error for invalid branchPoint state" in {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add why this is invalid here.

val wrongTree = dummyTracing.trees(1).copy(branchPoints = Seq(BranchPoint(99, 0)))
val newTracing = dummyTracing.copy(trees = Seq(dummyTracing.trees(0), wrongTree))

assert(true)//!isParseSuccessful(getParsedTracing(newTracing)))
//TODO: The parser currently doesn't check this
}

it should "throw an error for invalid edge state" in {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

val wrongTree = dummyTracing.trees(1).copy(edges = Seq(Edge(99, 5)))
val newTracing = dummyTracing.copy(trees = Seq(dummyTracing.trees(0), wrongTree))

assert(!isParseSuccessful(getParsedTracing(newTracing)))
}

it should "throw an error for edge with same source and target state" in {
val wrongTree = dummyTracing.trees(1).copy(edges = Edge(5, 5) +: dummyTracing.trees(1).edges)
val newTracing = dummyTracing.copy(trees = Seq(dummyTracing.trees(0), wrongTree))

assert(!isParseSuccessful(getParsedTracing(newTracing)))
}

it should "throw an error for duplicate edge state" in {
val wrongTree = dummyTracing.trees(1).copy(edges = Seq(Edge(4, 5), Edge(4, 5), Edge(5, 6)))
val newTracing = dummyTracing.copy(trees = Seq(dummyTracing.trees(0), wrongTree))

assert(!isParseSuccessful(getParsedTracing(newTracing)))
}

it should "throw an error for disconnected tree state" in {
val wrongTree = dummyTracing.trees(1).copy(edges = Seq(Edge(4, 5)))
val newTracing = dummyTracing.copy(trees = Seq(dummyTracing.trees(0), wrongTree))

assert(!isParseSuccessful(getParsedTracing(newTracing)))
}

it should "throw an error for duplicate tree state" in {
val newTracing = dummyTracing.copy(trees = Seq(dummyTracing.trees(0), dummyTracing.trees(0)))

assert(!isParseSuccessful(getParsedTracing(newTracing)))
}

it should "throw an error for duplicate node state" in {
val duplicatedNode = dummyTracing.trees(1).nodes(0);
val wrongTree = dummyTracing.trees(1).copy(nodes = Seq(duplicatedNode, duplicatedNode))
val newTracing = dummyTracing.copy(trees = Seq(dummyTracing.trees(0), wrongTree))

assert(!isParseSuccessful(getParsedTracing(newTracing)))
}

it should "throw an error for missing groupId state" in {
val wrongTree = dummyTracing.trees(1).copy(groupId = Some(9999))
val newTracing = dummyTracing.copy(trees = Seq(dummyTracing.trees(0), wrongTree))

assert(true)//!isParseSuccessful(getParsedTracing(newTracing)))
//TODO: The parser currently doesn't check this
}

it should "throw an error for duplicate groupId state" in {
val newTracing = dummyTracing.copy(treeGroups = TreeGroup("Group", 3) +: dummyTracing.treeGroups)

assert(true)//!isParseSuccessful(getParsedTracing(newTracing)))
//TODO: The parser currently doesn't check this
}
}
33 changes: 33 additions & 0 deletions test/backend/NMLWriterTestStub.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package backend

import javax.xml.stream.{XMLOutputFactory, XMLStreamWriter}

import com.scalableminds.util.geometry.Scale
import com.scalableminds.util.xml.Xml
import com.scalableminds.webknossos.datastore.SkeletonTracing.SkeletonTracing
import com.sun.xml.txw2.output.IndentingXMLStreamWriter
import models.annotation.nml.NmlWriter._
import play.api.libs.iteratee.Enumerator
import scala.concurrent.ExecutionContext.Implicits.global

object NMLWriterTestStub {
private lazy val outputService = XMLOutputFactory.newInstance()

def toNmlStream(tracing: SkeletonTracing, scale: Option[Scale]) = Enumerator.outputStream { os =>
implicit val writer = new IndentingXMLStreamWriter(outputService.createXMLStreamWriter(os))

val nml = Xml.withinElementSync("things") { writeTestSkeletonThings(tracing, scale)}
writer.writeEndDocument()
writer.close()
os.close
nml
}

def writeTestSkeletonThings(tracing: SkeletonTracing, maybeScale: Option[Scale])(implicit writer: XMLStreamWriter): Unit = {
Xml.withinElementSync("parameters")(writeParametersAsXml(tracing, "", maybeScale))
writeTreesAsXml(tracing.trees.filterNot(_.nodes.isEmpty))
Xml.withinElementSync("branchpoints")(writeBranchPointsAsXml(tracing.trees.flatMap(_.branchPoints).sortBy(-_.createdTimestamp)))
Xml.withinElementSync("comments")(writeCommentsAsXml(tracing.trees.flatMap(_.comments)))
Xml.withinElementSync("groups")(writeTreeGroupsAsXml(tracing.treeGroups))
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* Copyright (C) 20011-2014 Scalable minds UG (haftungsbeschränkt) & Co. KG. <http://scm.io>
*/
package frontend
package e2e

import scala.concurrent.{Await, Future}
import scala.sys.process.ProcessIO
Expand All @@ -28,6 +28,7 @@ class End2EndSpec(arguments: Arguments) extends Specification with LazyLogging {
("http.port" -> testPort,
"play.modules.disabled" -> List("com.scalableminds.webknossos.datastore.DataStoreModule"),
"play.http.router" -> "webknossos.Routes",
"insertInitialData" -> false,
"datastore.enabled" -> false)

"my application" should {
Expand Down