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 tests for adapter, business-delegate, factory-method and command modules #294

Merged
merged 5 commits into from Nov 22, 2015
Merged

Conversation

mfarid
Copy link

@mfarid mfarid commented Nov 18, 2015

This pull request contains unit tests for the following modules.

  • adapter
  • business-delegate
  • factory-method
  • command

The existing tests were more of Exercise Tests which were just running the code and printing the output. This code was not asserting the expected behaviour. This Pull Request covers the code to 100% of lines of these 4 patterns. The code coverage report can be generated and viewed using the instructions mentioned in CODE_COVERAGE.md

Please let me know in case of any issues/concerns.

Mohd Farid
DevFactory - Open Source Unit Test Team

DevFactory is committed to improving code quality of open source. Our team contributes their time to researching and identifying areas for improving test coverage of open source projects.

@iluwatar
Copy link
Owner

Here are my review remarks:

  • The code should be formatted to use Google coding conventions as described in the developer wiki
  • Do you think we still need the old AppTest classes? Does the coverage drop if we remove the old tests?
  • Thanks for the CODE_COVERAGE.md instruction. This is really useful :)
  • The code is missing JavaDocs describing the classes and tests

@iluwatar
Copy link
Owner

This is all for now. Please comment on this thread once you have made the changes and I will review again.

@mfarid
Copy link
Author

mfarid commented Nov 20, 2015

I have made the suggested changes. I look forward to your review comments.

@iluwatar
Copy link
Owner

Hi @mfarid I looked at it again. It is all great except that the code is still not formatted according to Google rules. Can you check once more?

@mfarid
Copy link
Author

mfarid commented Nov 22, 2015

I have formatted all these files using the suggested code style. Please check again and let me know if it is good now or needs any changes.

iluwatar added a commit that referenced this pull request Nov 22, 2015
Unit tests for adapter, business-delegate, factory-method and command modules
@iluwatar iluwatar merged commit 092d48d into iluwatar:master Nov 22, 2015
@iluwatar
Copy link
Owner

Looks good, thank you!

@faridiflex
Copy link

Thanks a lot for accepting this Pull Request. I appreciate your quick review and feedback.

@npathai
Copy link
Contributor

npathai commented Nov 23, 2015

Thanks for your contribution @faridiflex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants