kblincoe / QualOpt_SE701

2 stars 15 forks source link

#40 - Adding unit tests for GitHubResource REST controller and refactoring for Testability #136

Closed dylHall closed 6 years ago

dylHall commented 6 years ago

This contains the unit tests and necessary refactoring needed to test the functionality introduced by the first pull request for #40. This includes introducing a new class and making some changes to the original code to promote testability (ie. allowing for adding a mock implementation to avoid making GitHub API calls over the network during testing).

This completes the work needed for #40.

Please note: #41 will be impacted by this change, as refactoring has occurred to introduce the misdirection needed to allow Mockito to mock the method that is responsible for sending network calls. This change also depends on having #131 pulled in (had the Lob annotation for allowing mvnw test to work)

Summary of Changes

Summary of how Testing Works

The idea of the unit testing class introduced in this pull request is to set up the MockMVC controller with a default GitHubResource, and then each method is able to request a mock GitHubAPIService object (from the createMockGitHubAPIService() method).

These mocks simply override the behavior of the sendGitHubAPIUserSearch(), which consists of all the network calls to the GitHub API. It will simply do nothing or throw some kind of exception - this simulates either success or the various failures that could occur during execution.

Use of MockMVC allow testing of the web service aspects, ie. actually sending a POST request with the body of the request containing the GitHubAPIRequest (which is what is expected from the front-end).

The Mockito framework has been leveraged to provide the mocking capabilities - as it provides much more readable and maintainable code. No specific mocks classes have been made, as the framework is able to wrap the object and provide partial mocking capabilities.

If you have any questions about these changes, feel free to let me know.

dylHall commented 6 years ago

@crat019 @KijongHan could you have a quick review when you get the chance?

lilcham commented 6 years ago

@dylHall Looks good to me. All tests run and complete as intended. Good job!

screen shot 2018-03-27 at 10 43 16 am
dylHall commented 6 years ago

Tested with new JDK 9 changes and tests appear to be working all good!

dylHall commented 6 years ago

@softeng-701 this is PR is ready to be pulled in - I believe #41 relies on having these refactored changes as well.