OpenSRP Developer's Guide
Introduction to OpenSRP
OpenSRP is an open source mHealth platform for government frontline health workers who provide Reproductive, Maternal, Newborn, and Child Health (RMNCH) services in low-resource settings. The platform combines a simple, easy-to-learn tablet application with the health expertise of WHO and pragmatic approach of top field implementers. The code base is open to outside collaborators, with the current core technical collaborators located in Indonesia, Bangladesh, Pakistan, and Kenya.
Who should read this
If you want to commit code to any OpenSRP repository, you need to read this entire document.
Coding best practices
All OpenSRP code is hosted on GitHub at https://github.com/OpenSRP.
Write readable code.
Naming conventions
Classes - Class names always begin with a capital letter and if there are multiple words in the class name then each word must also begin with a capital letter, i.e. CamelCase.
Packages - Package names should always be in lowercase.
Objects/Variables - Instances and variables should start with lowercase and if there are multiple words in the name, then you need to use Uppercase for starting letters for the words except for the starting word. This is known as lowerCamelCase.
Methods - Methods in Java also follow the same lowerCamelCase convention like Objects and variables.
Constants - In Java constant variables are declared using “static final” modifiers. And such variables must contain only UpperCase characters and multiple words must be separated using ‘_’. Other constants that change based on build flavours, e.g. test, staging, prod should be specified in xml files for Android and .properties file in the webapps, e.g. db connection url.
Database tables - Relational database tables should be in lowercase, plural. If there are multiple words in the name, separate the words with an underscore. The corresponding Java model should be in singular and apply the classes naming conventions.
Third party libraries
External libs/jar files should only be added when it’s extremely necessary, e.g. adding Apache Commons lib in the Android code for the purposes of using classes like StringUtils is undesirable since this may lead to multidex issues coming up unnecessarily. Always check external library license details before including them in the project. If you are not sure about a library licensing interpretation, do not use it.
Imports
Do not use wildcard imports. If you are importing multiple classes from a package, list them all individually, like in the following code block, since this makes it clear exactly what imported classes are in use in any file.
import java.util.ArrayList;
import java.util.List;
import java.util.Random;
Do not do the following:
import java.util.*;
In eclipse, organize imports can be enabled to execute automatically on file save by enabling the feature in Project → Properties → Save Actions. This automatically gets rid of unused imports in a class.
Code formatting
It is recommended to use the default formatting feature of an IDE, e.g. Android Studio (AS), to format code. This generates consistently formatted code among developers, which in turn prevents unnecessary conflicts when merging code. To format code in AS use Ctrl+Alt+L (Option+Command+L for Mac). For eclipse use Ctrl+Shift+F (Command+Shift+F for Mac). In eclipse, code formatting can be enabled to execute automatically on file save by enabling the feature in Project → Properties → Save Actions.
Code commenting
Do not leave commented out code in the source files. Just remove it. There are some exceptions of course, such as if you're pretty sure it needs to be re-enabled at some point. To make sure you don't forget why that is, leave a clear TODO comment, e.g. "TODO this needs re-enabled when TRUNK-XXXX is fixed." Most IDEs are good at parsing out all TODO statements. When adding comments to the code follow the javadoc guidelines going forward to ensure that any developer can get the code and know what it does (remember we span out throughout the globe).
Exception handling
Use a given platform’s best exception logging infrastructure, e.g in Android, using Log.* classes is the recommended way of logging exceptions since exceptions logged this way can automatically be captured by crash reporting tools e.g Crittercism, Crashlytics. When creating new exception classes, create an intuitive hierarchy of exceptions within the package they belong, only creating new Exception classes when branching on different types of exceptions within client/webapp is needed (add as needed instead of making extra classes "just in case" for every possible variation). In Android, as a general rule, use the class name as tag and define it as a static final field at the top of the file. For example:
public class LoginActivity {
private static final String TAG = LoginActivity.class.getSimpleName();
public myMethod() {
Log.e(TAG, "error message");
}
}
VERBOSE and DEBUG logs must be disabled on release builds. It is also recommended to disable INFORMATION, WARNING and ERROR logs but keep them enabled wherever they may be useful to identify issues on release builds. If enabled, make sure that they are not leaking private information such as email addresses, user ids, etc.
Resources
Best practices for writing readable code: http://code.tutsplus.com/tutorials/top-15-best-practices-for-writing-super-readable-code--net-8118
How to write testable code: http://www.toptal.com/qa/how-to-write-testable-code-and-why-it-matters
Code readability test: http://www.clock.co.uk/blog/code-readability-the-5-minute-explanation-test
Javadoc standards: http://docstore.mik.ua/orelly/java-ent/jnut/ch07_03.htm
Code testing in OpenSRP
OpenSRP adopts Test Driven Development (TDD), which is an evolutionary approach to development that combines test-first development where we write a test before we write just enough production code to fulfill that test and refactoring. It’s one way to think through your requirements or design before we write our functional code (implying that TDD is both an important agile requirements and agile design technique).
Using this process, you write an automated test first, ensure that the test fails, write the code, and then ensure that the test passes. The benefits of the TDD process are that it drives the API and design, provides good code coverage, promotes safe code refactoring, and keeps developers focused on the task.
Generally, following TDD methodology, the developer will:
Create a feature branch
Write failing high-level tests that fail and commit (with a well defined commit message) this to the new branch.
Start working on the implementation code as the code is continuously committed to the feature branch
When the feature is finished, update and run the tests to make sure they pass
Merge the feature branch into the release branch
Unit tests
The goal of unit testing is to isolate each part of the program and show that the individual parts are correct. A unit test should address a single point of functionality and should be fast, understandable (by a human), automatic (no human intervention), and isolated from external resources such as a database, file system, web service, and JMS broker etc.
Unit tests are useful in that:
They allow you to check the expected behavior of the piece(s) of code you are testing, serving as a contract that it must satisfy.
They also allow you to safely refactor code without breaking the functionality (contract) of it.
They allow you to make sure that bug fixes stay fixed by implementing a Unit test after correcting a bug.
They may serve as as a way to write decoupled code (if you have testing in mind while writing your code).
Integration testing
Integration tests address another aspect of testing where we test software modules in realistic production conditions to validate overall functionality. As a white-box strategy, integration testing is aware of internal implementation details. This time, we use a real database, real API calls and mocking is generally discouraged in these tests.
Since integration tests use a real database, extreme care should be taken to make sure that the tests are not set to run on production databases or resources. To handle this a test.properties file should be setup with the test resources definitions and the test build should also be made to fail if not provided with the build environment params. I.e. test builds should always check or automatically switch the properties file to test.properties.
How to distinguish integration tests from unit tests:
a test uses the database
a test uses the network
a test uses an external system (e.g. a queue or a mail server)
a test reads/writes files or performs other I/O
Server side integration testing
The Spring Framework provides support for integration testing with the Spring TestContext Framework, which lets you write tests for a framework like JUnit. In JUnit, the @RunWith annotation sets a Runner class, which is responsible for launching tests, triggering events, and marking tests as successful or failed
For server side integration testing, use the following Spring TestContext Framework features:
The
@ContextConfiguration
annotation is used to load a Spring context from an XML file, for example,@ContextConfiguration("/path/context.xml")
. By convention, if an application context path isn’t set, the path is set to [Test- ClassName]-context.xml.Spring TestContext caches the application context for better performance. This saves time on each test startup.
The
@DirtiesContext
annotation indicates that the application context for a test is dirty and that TestContext should close it after each test.Spring TestContext defines a listener API for tests to interact with objects with the
@TestExecutionListeners
annotation. For example, the DependencyInjectTestExecutionListener class provides support for dependency injection and initialization of test instances. Spring TestContext adds this listener by default and injects field by type, annotated with@Autowired
or by name with@Resource
.
The following simple example uses these features:
@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration("/path/to/context.xml")
public class OpenSRPIntegrationTest {
@Autowired
private OpenSRPObject object;
@Test
public void test() {
}
}
Functional testing
Functional tests focus on the functional requirements of an application, and use the black-box testing strategy. This involves providing input values and verifying output values without concern for any implementation details. Functional testing ensures that the app meets its functional requirements and achieves a high standard of quality such that it is more likely to be successfully adopted by users.
OpenSRP-client (Android)
Functional tests should be setup using Espresso to simulate user interaction using ViewMatchers class, Hamcrest Matchers or the onData method to locate view items.
OpenSRP-server(Java, HTML)
Functional tests should be setup using Selenium to simulate user interaction.
For both server and client apps, if a click on a view results in the creation, updating, or deletion of records in the database, the respective test should test for this change as well. E.g A test for the login page should test that all the required views are visible and respond to the clicks by either displaying validation error messages or navigating to a new screen and also validate that user data is saved in the sharedpreferences or in the local client/server database.
As a good practice and for code reuse and maintainability, all view access code in a given page should be put in a pageobject.
Continuous integration
OpenSRP uses Travis as its continuous integration server. On both our OpenSRP server and client, we have set up the .yml file, which calls the tests build tool.
On the OpenSRP server, we use the Maven build tool to build our code dependencies. Therefore, on the travis.yml file, we call Maven tests as shown below:
- mvn test
Testing conventions
To implement a test case, start by creating a public class; by convention, post- fix the class name with Test or TestCase and let the name be prefix be similar to the class being tested, for example, SignInServiceTest. Because we use annotations, test class no longer needs to inherit from the JUnit TestCase class as it did in JUnit version 3. In Android every Espresso test class usually targets an Activity, therefore the name should match the name of the targeted Activity followed by Test, e.g. LoginActivityTest.
Sometimes a class may contain a large amount of methods, that at the same time require several tests for each method. In this case, it's recommended to split up the test class into multiple ones. For example, if the SignUp class contains a lot of methods we may want to divide it into SignUpResetAccountsTest, SignUpRegisterUserTest, etc. Generally, you will be able to see what tests belong together, because they have common test fixture.
To create a test method, you write a public method with no return value, no arguments, and a
@Test
annotation. By convention, the method name starts with the name of the method that is being tested. i.e.Template
@Test void methodNamePreconditionExpectedBehaviour()
Example
@Test void signInWithEmptyEmailFails()
Note: Precondition and/or expected behaviour may not always be required if the test is clear enough without them.
At the start of a test run, JUnit/TestNG runs methods with
@Before
annotations before each test method. Use@Before
methods to initialize new instances of test objects stored in instance variables. These objects are available in each test method, and are called fixtures. JUnit runs methods with@After
annotations after each test method to clean up fixtures or other resources as necessaryBecause some resources are expensive to initialize and manage, you can have JUnit call setup and teardown methods once per test run. Use the
@BeforeClass
annotation to call a method once per test run before JUnit has run all test methods. Use the@AfterClass
annotation to call a method once per test run after JUnit has run all test methodsUse Methods in the JUnit Assert class to help you compare an expected value to an actual value. If an assert method contract is respected, the test continues; if not, the method throws an Error, and the test is stopped and marked as failed
Testing best practices
At least one test class per new class
All tests should be granular i.e., a single test should test for a single condition or outcome only. E.g. it’s not recommended to have if statements in tests, every condition in a method should have its own test method in a test class.
Every test must use
Assert.assert___
methods to test output. DO NOT useprintln
statements!If committing new code, the tests for that code should be committed at the same time
If fixing a bug, write a failing test proving the bug, then fix the code to make the test pass.
Every unit test method should be well documented (javadoc) about what it is expected and where it is operating on.
Every database-based test must be accompanied by a dbunit test dataset.
Always remember to test for exceptions for methods that throw Exception
For a complex object, you only want to verify the behavior of the object, not the dependencies the object relies on. To achieve this goal, create mock objects for dependencies. E.g. when writing a unit test for a method that makes an expensive API call to other services, consider mocking the call to the other services.
Tests that manipulate db/file data or any other shared resource should reset the resources on exit
Tests shouldn’t have try/catch but instead should throw exceptions so that the tests report failures when an exception occurs. In the very special cases where a try/catch is required, in the catch block add Assert.fail(“”).
Resources
Android UI Testing
http://developer.android.com/training/testing/ui-testing/index.htmlAndroid Unit Testing
http://developer.android.com/training/testing/unit-testing/index.htmlSelenium
http://www.seleniumhq.org/projects/webdriver/
https://github.com/SeleniumHQ/selenium/wiki/PageObjectsMockito
http://mockito.org/
Milestones
OpenSRP milestones are one week durations - they start each Wednesday morning and go for two weeks, till COB on Tuesday of the 2nd week. Milestones are labeled with the OpenSRP version number, the milestone number, and the date of the last day of the milestone. E.g. V2 -1- 04 Aug 2015 means this milestone falls under the version 2 development tasks, it’s the first milestone and the milestone is due by COB on 04 August 2015 (a Tuesday).
The current milestone contains work expected to be completed in the current week. Later milestones show upcoming work, or they show work that may take longer than 2 weeks.
When to change an issue's milestone
If you run out of work in the current milestone, pull up an issue from the next milestone, based upon prioritization, and make a comment.
If you realize you cannot complete your assigned issues in the current milestone, bump an issue down, based upon prioritization, and make a comment.
If you think there is an error in the prioritization, e.g. dependencies/blockers, production errors, user request, make a comment and talk to a Project Manager.
Issues
We use GitHub issues to track development tasks, bugs and documentation. All issues should be labeled as appropriate, assigned to someone, and given a milestone. All discussion about an issue should take place (or be transferred to) comments in the issue. Details, plans, and questions on implementation should be put in comments as well.
Sizing issues
All issues should have a size label of:
Small (< 1 week)
Medium (~1 week)
Large (1 - 2 weeks)
Extra large (2+ weeks)
NB: Before assigning Large or Extra large labels, explore possibilities of splitting the issue into smaller, more manageable tasks.
If you think an issue is the wrong size, change the label and make a comment.
If the issue is taking longer than the size the label says, change the label and make a comment that will assist the team to correctly determine the size.
If you need help sizing an issue, ask someone.
Only create issues that can be closed.
Prioritization
Prioritization of tasks is determined by two factors:
Milestone: issues in the current milestone should be completed before issues in later milestones
The Priority labels: issues within the same milestone can be prioritized using the Low, Medium, and High Priority labels
NB: There is a third factor that affects prioritization: specific directions communicated by a Project Manager. This communication should be immediately translated into a milestone and appropriate labels.
Blockers
If an issue depends on another open issue, feedback from a stakeholder, or discussion with the tech/science team, it is considered blocked.
Add the Blocked label and make a comment. If it is blocked by another issue, reference that issue in your comment.
If all blockers are removed, i.e. you close an issue that was blocking another issue, remove the Blocked label on the other issue and make a comment.
Errors and deploy blockers
Reports of and found errors (bugs are errors) should get the Bug label.
Critical errors and any regressions (things that used to work but are now broken) need to be fixed before a deploy, these get the Blocks Deploy label.
Branches
We use GitHub Flow for our code control. GitHub Flow is a lightweight, branch-based workflow that supports teams and projects where deployments are made regularly. If you are unfamiliar with GitHub Flow, please review the above link carefully to become familiar with the steps.
Branches should be named according to the issue or issues (refer to the issues section above) it relates to in order to make it very easy for other developers to see what is currently being worked on, except for release branches. For example, on the OpenSRP client repo, if you are working on issue #166, then your work should be in a corresponding branch named issue166. If there is no existing issue for what you want to work on, then first create the issue, then the branch.
No one should be opening branches in personal or private repositories - all OpenSRP-related work should go in the OpenSRP source code repository.
Rule of thumb: Always be updating your branch with the latest master changes to avoid having conflicts when merging back to the production branch.
Commits
First, be aware of ABC - Always Be Committing! Make frequent and small commits.
Second, the 7 rules of
Separate subject from body with a blank line
Limit the subject line to 50 characters
Capitalize the subject line
Do not end the subject line with a period
Use the imperative mood in the subject line
Wrap the body at 72 characters
Use the body to explain what and why vs. how
Now some OpenSRP specifics:
Prefix all commits and pull requests with the initials of the person(s) who worked on it
When you see a commit or pull request, you should easily know who to ask to get more details about what's going on. To make this easy at a glance, prefix all commits and pull requests with the initials of the person or persons who worked on it, with multiple people delimited using a pipe (|). For example:
DC|RA: Migrate to gradle
MK: Remove live server calls from connector and web
Your initials should be the first characters of your first and last name, you can make an exception as along as it is 2-3 characters and it is consistent, easily attributable to you, and always the same 2-3 characters.
Include the issue number in the commit message, this enables GitHub to include the commit messages in the issue’s page
DC|RA Fixed Issue: Migrate to gradle #212
Closing issues via commit messages
To close an issue in the same repository, use one of the keywords: close, closes, closed, fix, fixes, fixed, resolve, resolves, resolved followed by a reference to the issue number in the commit message. For example, a commit message with fixes #45
will close issue 45 in that repository once the commit is merged into the default branch.
Rule of thumb:
Don’t commit broken code
Always commit at the end of the day before leaving the office
Don’t commit untested code
Don’t commit when the build is broken
Don’t go home after committing until the system builds and there are no broken features.
e.g. for version source control
https://blog.rainforestqa.com/2014-05-28-version-control-best-practices/
https://github.com/torvalds/linux/pull/17#issuecomment-5659933
Pull Requests (PRs)
Everyone should open pull requests early and aggressively to get others looking at your work and get feedback. A good rule of thumb is to have one PR per issue. This leads to two types of pull requests:
Pull requests that can be merged
Pull requests that can not (or should not) be merged
The title should succinctly describe the issue it is closing (including the issue number) and the solution chosen. If it is the second type of PR and cannot be merged, the title of the pull request needs to be prefixed with [DO NOT MERGE], for example:
[DO NOT MERGE] 63 Remove live server calls from connector and web
For either type of PR, the description should follow the below format:
New Features - list the new features and/or functionalities that are being added
Changes - list any changes that were made to existing code
Fixes - list the issues that are being fixed with this PR
Known Issues - list any remaining known issues that are still not resolved
Someone other than the original author should review a PR prior to merging. One should never merge one’s own PRs unless absolutely necessary. All PRs should be merged within the same milestone they were created.
What to look out for before merging a pull request:
Tests to confirm that the code changes are fine (include the full test class package and name)
Well commented and formatted code
No conflicts
To effectively merge a pull request the following procedure should be followed:
Assume you want to merge a pull request for a branch called issue212:
First, make sure you have the latest master code in your local repo.
git checkout master
git pull origin master
Then, pull the changes to your local repo and switch to the pull request branch.
git fetch origin
git checkout -b issue212 origin/issue212Merge master into the pull request branch.
git merge master
Test the code for conflicts, build errors, failing tests and most importantly confirm that the issue has been fixed. At this point you could use the github user interface to merge the pull request and skip step 5.
Switch back to master and merge the changes from the pull request branch and push the changes.
git checkout master
git merge --no-ff issue212
git push origin masterClose the pull request.
Delete the remote branch
git push origin :issue212
Peer Reviews (Code Reviews)
Every time a feature is updated or a bug is fixed, a Peer review or Code review is required before the code/patch is merged to master.
The Developer is required to:
- Create a PR(Pull Request) to the parent branch, usually master. However it could any other depending on the project set up e.g. develop branch
- Assign 1 or 2 reviewers to the PR
- Notify them. While Github does have notifications for the above actions, it is recommended to notify the reviewers you assigned especially depending on urgency
The Reviewer is required to:
- Ensure all the CI status checks as set up on Github pass e.g. Build, Codacy, Code coverage
- Perform a manual review of the code. If anything requires an update, add a comment to the corresponding lines on the file within the Github PR
- Check out the code on your local work station, deploy and :
- Test the new feature while referencing the corresponding spec/Github issue
- Test any other related feature the update might affect
- Test for any possible backward compatibility breaks
If all the above pass, then the reviewer approves the PR. Note that though Github does have notifications for the above actions, It is recommended to notify the requesting developer once you are done reviewing incase the update/patch is an urgent one.
The developer requesting the PR then follows these steps:
- Merge the PR code from the issue branch to master
- Delete the issue branch
In the case that the update required a release/snapshot, the developer is also required to:
- Bump up the library/app version (Should be part of the PR above)
- Push a signed tag with the new version number and info on the update in the description as described here How to upload Android client libraries to Maven/Sonatype
- Upload the artifact to Maven if its a library
NB: If its a release (as opposed to a Snapshot), then create a Release on Github as described here https://help.github.com/en/articles/creating-releases. (Uploading artefacts with the tags is optional) and also check out the OpenSRP guidelines here How to create a release APK
Release management
The release tag for major releases should also have an accompanying text document with complete, clear release notes for the code released. This release document should also be shared with the other developers through the mailing list.
Also check out How to create a release APK
This site is no longer maintained. Please visit docs.opensrp.io for current documentation.