Skip to content

DM-8854:Create a Unit test for Rotate class #342

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

Merged
merged 3 commits into from
Apr 4, 2017
Merged

Conversation

lznakano
Copy link
Contributor

DM-8854:
Add unit test for Rotate class
Add the algorithm descriptions

The coverage is 80%. There is a main program inside which made the coverage lowered. To
test it, please run:
gradle :firefly:test

Thanks!

  Add unit test for Rotate class
  Add the algorithm descriptions

DM-8854:
  Add more unit tests
@lznakano lznakano requested a review from cwang2016 March 31, 2017 17:48
*
* Ry(angle) = {
*
* cos(angle) 0 - sin(angle)
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be -sin(angle) or +sin(angle)?

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, I have the correct document in RotateTest.java but Roate.java. There is nothing documented there when I started working on this ticket. I added the algorithm in RotateTest.java. I also added some in Rotate.java. I created another ticket for Rotate.java. I will update the document and the code then.

@cwang2016
Copy link
Contributor

cwang2016 commented Apr 3, 2017

review complete. Here is some concern,

  • the unit test contains the computation to calculate the expected output used for baseline, how would this piece of computation to be verified? The class to be tested, rotate.class, intends to build rotation matrix for the purpose to rotate any given world point to be aligned with x axis at 180 deg direction (as I traced its code), so the unit test should just check if input point used for the baseline is rotated to (-1, 0, 0) while testing 'do_rotate' function in Rotate.class. Since the functional description is not seen from the document of the tested class, so it is understandable why the algorithm is coded and put inside the unit test.

You may merge the branch. Thanks.

Lijun Zhang added 2 commits April 4, 2017 09:45
  Add another unit test and commented out the algorithm verfication.
  Add the documentation in the Rotate.java
@lznakano lznakano merged commit 23d41c5 into dev Apr 4, 2017
@lznakano lznakano deleted the DM-8854-UnitTestForRotate branch April 4, 2017 17:13
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.

2 participants