nikhilvharamble / bus-stop-alarm

Automatically exported from code.google.com/p/bus-stop-alarm
MIT License
0 stars 0 forks source link

Code reviews for ConfirmationPageTests.java #29

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Copy the ConfirmationPageTests.java to src/ folder of the android test
project

Original issue reported on code.google.com by QUANGHUY...@gmail.com on 23 Feb 2010 at 8:22

Attachments:

GoogleCodeExporter commented 9 years ago
CODE REVIEW - Bus Stop Alarm
Date: Feb 24, 2010
File: ConfirmationPageTests.java (in modules folder)
Revision: 122
Author: Huy Dang
Reviewer: Derek Cheng

Summary:

This is a first review of ConfirmationPageTests.java. This file contains unit 
tests 
for the GUI part of the ConfirmationPage class, which was reviewed earlier. 
This file 
has been newly added by the author, and the entire file will be the focus of 
this 
review. This review will focus primarily on code formatting, the use of 
comments, 
adherence to Java / Android style guidelines, and unit test guidelines as per 
The Art 
of Unit Testing.

Comments:

CODING STANDARDS
Many of the tests cases get some part of the GUI (button, seek bar) by calling 
findViewById(). The initialization of these parts takes up 2 lines. That is 
fine, but 
putting the casting also on the second line would make it look much more 
clearer to 
those who read the code. 

COMMENTS
Author and file description are present on top of file. Sufficient Javadocs 
style 
comment are present on top of each test case. 

LOGIC
Tests independent of each other, one assertion per test, seems to be following 
the 
guidelines.
There are some assertions that are done with assertEquals(..., true/false, 
...). It 
would be better to use assertTrue() and assertFalse() instead.

ERROR HANDLING
N/A

CODE DECISIONS
There seems to be extensive test coverage for testing the functionality of 
buttons 
and seekbars, which make up the the GUI of the confirmation page.

Status:

Ready to check in once changes are made.

Original comment by chen...@gmail.com on 25 Feb 2010 at 3:05

GoogleCodeExporter commented 9 years ago
Since we failed to set up the test project for current project and couldn't 
make it 
work for other after uploading to svn. We had to manually upload the file here 
so 
assigned person could do code review. After that, we figured what the problem 
is and 
test project is checked out successfully from svn
Issue updated:
What steps will reproduce the problem?
. Check out the current newest version of ConfirmationPageTests.java based on 
the 
date this issue is created.
2. Code review requested for this file

Expected:
- all code should follow the coding style guideline
Actual:
- there're a few errors in coding style that will probably be pointed by code 
review

Original comment by QUANGHUY...@gmail.com on 14 Mar 2010 at 4:55