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

Conversation

youri-k
Copy link
Contributor

@youri-k youri-k commented Jun 29, 2018

  • backend now has unit-tests for nml parser and writer
  • they work similar to the frontend tests

Steps to test:

  • call sbt "test-only *NMLUnitTestSuite" in wK directory to see the tests running

Issues:


  • Ready for review

@youri-k youri-k self-assigned this Jun 29, 2018
@youri-k
Copy link
Contributor Author

youri-k commented Jun 29, 2018

@jstriebel these tests currently run in the e2e-block of tests. Could you configure that differently?

@jstriebel
Copy link
Contributor

@youri-k, do you mean the docker-compose service e2e-tests? I can split that into e2e-tests and backend-tests, yes. The unit tests don't need fossildb or postgres, right?

@youri-k
Copy link
Contributor Author

youri-k commented Jun 29, 2018

Right
Thanks 🙂

@youri-k youri-k changed the title [WIP] backend unit tests for nml parser and writer backend unit tests for nml parser and writer Jun 29, 2018
@jstriebel
Copy link
Contributor

@youri-k: I just added a docker-compose service backend-tests and made sure that the e2e tests don't execute them. Also the CI runs them now.

Copy link
Contributor

@jstriebel jstriebel left a comment

Choose a reason for hiding this comment

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

Awesome PR, testing this makes so much sense! 🎉

Only left some minor remarks, besides that LVGTM 👍

@@ -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


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.

}
}

"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.

//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.

//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

Copy link
Contributor

@jstriebel jstriebel left a comment

Choose a reason for hiding this comment

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

👍 🎉

@youri-k youri-k merged commit 9436584 into master Jul 9, 2018
@philippotto philippotto deleted the backend-unit-tests branch August 13, 2018 11:43
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.

backend nml writer + parser unit test
2 participants