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

Unit Testing for sdl_android Project #88

Closed
wants to merge 50 commits into from

Conversation

crokita
Copy link

@crokita crokita commented Jan 29, 2015

💥 READY FOR REVIEW 💥

Related to issue #17 .

Summary

This is the first-round of unit tests for the Android mobile libraries. More unit tests will be written in the future and unit tests should be updated whenever an API change occurs.

This pull request adds unit testing to RPC classes. The code was originally pulled from GENIVI's unit test branch at smartdevicelink_android.git

This commit tests all RPC objects. Each RPC object should test every available API method and ensure they work correctly. Currently, failed tests are related to #11, #52, #71 and #80.

Unit tests should be modified as additional code is added to the project and unit tests should be run on all code prior to an official release (this should happen on the release branch). Any issues that arise should have a unit test written for it showing failure before the issue is fixed and passed after the fix is applied.

@mikeburke106 mikeburke106 added REVIEW - scheduled The reviewer has scheduled a review of this PR for the current sprint and removed REVIEW - scheduled The reviewer has scheduled a review of this PR for the current sprint labels Jan 29, 2015
@mikeburke106 mikeburke106 changed the title Unit Testing for sdl_android Project [WIP] Unit Testing for sdl_android Project Jan 29, 2015
@mikeburke106 mikeburke106 changed the title [WIP] Unit Testing for sdl_android Project Unit Testing for sdl_android Project Jan 29, 2015
@mikeburke106 mikeburke106 added the REVIEW - scheduled The reviewer has scheduled a review of this PR for the current sprint label Jan 29, 2015
@mikeburke106 mikeburke106 added this to the 4.0.0 milestone Jan 30, 2015
@mikeburke106 mikeburke106 removed the REVIEW - scheduled The reviewer has scheduled a review of this PR for the current sprint label Jan 30, 2015
@mikeburke106
Copy link
Contributor

Here are the results of the unit tests for this PR (tested on master at the time this repo was forked). Note that this PR needs to be re-based, so some issues have recently been fixed but are still showing failures. These failed tests should pass after re-basing the branch.

Tests Run Test Passes (%) Test Failures (%) Test Errors (%)
1453 1306 (90%) 103 (7%) 44 (3%)

The difference between a test failure and a test error is that a failure successfully runs through the test code, but at least one of the test assertions was failed, while a test error crashes while executing the test code due to an exception that wasn't caught by the test.

Of the 147 failed tests, 75 are related to defensive-copying (#11), 17 are related to enums not handling valueForString consistently (#91), 32 are resolved issues that need to be re-based, 10 are bugs in the mobile libraries (#71 & #102) and the remaining 13 are issues with the tests themselves.

Livio is working to fix the test errors and get the tests to a state where all failures represent real issues that are posted to Github issues. We are also working on increasing unit test coverage as far as possible.

@mikeburke106 mikeburke106 mentioned this pull request Feb 9, 2015
@mikeburke106 mikeburke106 added the REVIEW - scheduled The reviewer has scheduled a review of this PR for the current sprint label Feb 23, 2015
@mikeburke106 mikeburke106 modified the milestones: 4.0.0, 3.4.0 Feb 23, 2015
@mikeburke106
Copy link
Contributor

Adding unit tests should really be a hotfix, so I'm pushing this up from v4.0.0 to v3.4.0. We should get this in ASAP so new PRs can unit test their changes more easily.

RPC classes and enums are thoroughly tested so far. All tests are either passed or failed with bug reported in Github.

@mikeburke106
Copy link
Contributor

Has anyone reviewed the existing unit tests at all? We have about 1,500 written so far, but I would like to know any corner cases we might be missing that should be tested for.

Right now, we're only testing RPC classes & enums because they are well defined and encapsulated in the code, but we definitely need to keep adding test cases. Some pieces of the code will be practically impossible to unit test due to statefulness, dependencies and architecture issues, but we should identify some other classes that could be unit tested.

@mikeburke106
Copy link
Contributor

Need to do the following before this can be merged:

  • Edit contributing document stating that all new pull requests must include unit tests or receive an exception from maintainers to omit unit tests
  • Add instructions for developers letting them know they don't need to include the unit tests in their project and the tests will not add to their APK size
  • Instructions to run tests in eclipse
  • Create a command line script to run the unit tests, save results to file and/or email results

@mikeburke106 mikeburke106 modified the milestones: 4.0.0, 3.4.0 Mar 9, 2015
@joeygrover
Copy link
Member

Closed due to another PR being used.

@joeygrover joeygrover closed this May 19, 2015
@joeygrover joeygrover added REVIEW - rejected The reviewer has rejected this PR and it will not be merged and removed REVIEW - scheduled The reviewer has scheduled a review of this PR for the current sprint labels May 19, 2015
@joeygrover joeygrover mentioned this pull request Aug 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
REVIEW - rejected The reviewer has rejected this PR and it will not be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants