diff --git a/source/api.tests/Weather/ApixuClientTests.cs b/source/api.tests/Weather/ApixuClientTests.cs index 35fe379..2716fd0 100644 --- a/source/api.tests/Weather/ApixuClientTests.cs +++ b/source/api.tests/Weather/ApixuClientTests.cs @@ -1,9 +1,7 @@ using Api.Weather; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; -using Moq.Protected; using RichardSzalay.MockHttp; -using System; using System.Net.Http; using System.Threading.Tasks; @@ -15,64 +13,89 @@ public class ApixuClientTests { private (ApixuClient apiuxClient, MockHttpMessageHandler mockHttp) Factory() { - var fakeHttpClient = new Mock(); - - // using https://github.com/richardszalay/mockhttp + // using https://github.com/richardszalay/mockhttp not var fakeHttpClient = new Mock(); var mockHttp = new MockHttpMessageHandler(); var httpClient = mockHttp.ToHttpClient(); var apiuxClient = new ApixuClient(httpClient); return (apiuxClient, mockHttp); } - [TestMethod] - public async Task ApixuClientTests_GetTemp_GivenAZipCode_ReturnsTemp() - { - // Arrange - var (apiuxClient, mockHttp) = Factory(); - const int zipCode = 57105; - const double fakeTemp = 70.5; - var response = new ApiuxWeatherCurrentResponse - { - Current = new ApiuxWeatherCurrent - { - TempF = fakeTemp - } - }; - var requestUri = $"https://api.apixu.com/v1/current.json?key={ApixuClient.apiKey}&q={zipCode}"; - mockHttp.When(requestUri) - .Respond("application/json", Serialize.ToJson(response)); - // Act - var result = await apiuxClient.GetCurrentTempAsync(zipCode); + //[TestMethod] + //public async Task ApixuClientTests_GetTemp_GivenAZipCode_ReturnsTemp() + //{ + // // Arrange + // var (apiuxClient, mockHttp) = Factory(); + // const int zipCode = 57105; + // const double fakeTemp = 70.5; + // var response = new ApiuxWeatherCurrentResponse + // { + // Current = new ApiuxWeatherCurrent + // { + // TempF = fakeTemp + // } + // }; + // var requestUri = $"https://api.apixu.com/v1/current.json?key={ApixuClient.apiKey}&q={zipCode}"; + // mockHttp.When(requestUri) + // .Respond("application/json", Serialize.ToJson(response)); - // Assert - Assert.AreEqual(fakeTemp, result); - } + // // Act + // var result = await apiuxClient.GetCurrentTempAsync(zipCode); - [TestMethod] - public async Task ApixuClientTests_GetTemp_GivenAZipCode_UsesThatZipCode() - { - // Arrange - var (apiuxClient, mockHttp) = Factory(); - const int zipCode = 57105; - const double fakeTemp = 70.5; - var response = new ApiuxWeatherCurrentResponse - { - Current = new ApiuxWeatherCurrent - { - TempF = fakeTemp - } - }; - var requestUri = $"https://api.apixu.com/v1/current.json?key={ApixuClient.apiKey}&q={zipCode}"; - var request = mockHttp.When(requestUri) - .Respond("application/json", Serialize.ToJson(response)); + // // Assert + // Assert.AreEqual(fakeTemp, result); + //} - - // Act - var result = await apiuxClient.GetCurrentTempAsync(zipCode); + //[TestMethod] + //public async Task ApixuClientTests_GetTemp_GivenAZipCode_NotFound_ThrowsException() + //{ + // // how do we handle error states outside our control? + // // Arrange + // var (apiuxClient, mockHttp) = Factory(); + // const int zipCode = 57105; + // const double fakeTemp = 70.5; + // var response = new ApiuxWeatherCurrentResponse + // { + // Current = new ApiuxWeatherCurrent + // { + // TempF = fakeTemp + // } + // }; + // var requestUri = $"https://api.apixu.com/v1/current.json?key={ApixuClient.apiKey}&q={zipCode}"; + // var request = mockHttp.When(requestUri) + // .Respond(HttpStatusCode.NotFound); - // Assert - Assert.AreEqual(1, mockHttp.GetMatchCount(request)); - } + // // Act and Assert + // await Assert.ThrowsExceptionAsync(async () => + // { + // await apiuxClient.GetCurrentTempAsync(zipCode); + // }); + //} + + //[TestMethod] + //public async Task ApixuClientTests_GetTemp_GivenAZipCode_UsesThatZipCode() + //{ + // // Arrange + // var (apiuxClient, mockHttp) = Factory(); + // const int zipCode = 57105; + // const double fakeTemp = 70.5; + // var response = new ApiuxWeatherCurrentResponse + // { + // Current = new ApiuxWeatherCurrent + // { + // TempF = fakeTemp + // } + // }; + // var requestUri = $"https://api.apixu.com/v1/current.json?key={ApixuClient.apiKey}&q={zipCode}"; + // var request = mockHttp.When(requestUri) + // .Respond("application/json", Serialize.ToJson(response)); + + + // // Act + // var result = await apiuxClient.GetCurrentTempAsync(zipCode); + + // // Assert + // Assert.AreEqual(1, mockHttp.GetMatchCount(request)); + //} } } diff --git a/source/api.tests/Weather/MockHttpClientHandlerExtensions.cs b/source/api.tests/Weather/MockHttpClientHandlerExtensions.cs index 0dd46e4..bebdd05 100644 --- a/source/api.tests/Weather/MockHttpClientHandlerExtensions.cs +++ b/source/api.tests/Weather/MockHttpClientHandlerExtensions.cs @@ -1,12 +1,4 @@ -using Moq; -using Moq.Protected; -using System; -using System.Net; -using System.Net.Http; -using System.Threading; -using System.Threading.Tasks; - -namespace Api.Tests.Weather +namespace Api.Tests.Weather { // use https://github.com/richardszalay/mockhttp instead /// @@ -16,13 +8,13 @@ namespace Api.Tests.Weather //{ // public static void SetupGetStringAsync(this Mock mockHandler, string response, Uri requestUri) // { - // if you have to be specific or multiple calls in the same method change or want to verify it - //mockHandler.Protected() - // .Setup>("SendAsync", - // ItExpr.Is(message => message.RequestUri == requestUri), - // ItExpr.IsAny()) - // .Returns(Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(response) })) - // .Verifiable(); + // if you have to be specific or multiple calls in the same method change or want to verify it + //mockHandler.Protected() + // .Setup>("SendAsync", + // ItExpr.Is(message => message.RequestUri == requestUri), + // ItExpr.IsAny()) + // .Returns(Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(response) })) + // .Verifiable(); // } //} } diff --git a/source/api.tests/Weather/WeatherControllerTests.cs b/source/api.tests/Weather/WeatherControllerTests.cs index 1b41ca1..7bb7dd9 100644 --- a/source/api.tests/Weather/WeatherControllerTests.cs +++ b/source/api.tests/Weather/WeatherControllerTests.cs @@ -2,8 +2,6 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; -using System; -using System.Collections.Generic; using System.Threading.Tasks; namespace Api.Tests.Weather @@ -14,36 +12,96 @@ public class WeatherControllerTests { private (WeatherController weatherController, Mock getWeatherHttpClient) Factory() { + // an alternative to returning the tuple, is to have private properties on this class for the injected classes + // this is useful if you're class/unit under test has too many dependencies + // which means you should probably refactor that class to separate out concerns var getWeatherHttpClient = new Mock(); return (new WeatherController(getWeatherHttpClient.Object), getWeatherHttpClient); } [TestMethod] - public async Task WeatherController_GetCurrentTemp_NoZipCode_Returns400() + public async Task WeatherController_CurrentTemp_NoZipCode_Returns400() + { + // Given an API call + // When asking for current temp and no zip code is given + // Then returns a 400 + Assert.Fail(); + } + + #region first tests + /* + // let's start with the not allowed use case, those are easy to think of + // (work as a team with QA to come up with these before you start coding + // at least before you consider your work as done) + [TestMethod] + public async Task WeatherController_CurrentTemp_NoZipCode_Returns400() { // Arrange var (weatherController, getWeatherHttpClient) = Factory(); + // we won't get to the client call, so no stubs setup needed + + // Act + var result = await weatherController.CurrentTemp(0); + + // Assert + Assert.AreEqual(400, (result.Result as BadRequestObjectResult).StatusCode); + } + */ + /* + [TestMethod] + public async Task WeatherController_CurrentTemp_NoOrBadZipCode_Returns400() + { + //Given an API call + //When asking for current temp and no zip code is given + //Then returns a 400 + // Arrange + var (weatherController, getWeatherHttpClient) = Factory(); + // Act var result = await weatherController.CurrentTemp(0); // Assert Assert.AreEqual(400, (result.Result as BadRequestObjectResult).StatusCode); } + */ + /* + // now, let's learn how to use DataRow to get more coverage + // and replace WeatherController_CurrentTemp_NoOrBadZipCode_Returns400 + [TestMethod] + [DataRow(0)] + [DataRow(-57105)] + [DataRow(12)] + [DataRow(1921393)] + public async Task WeatherController_CurrentTemp_NoOrBadZipCode_Returns400(int zipCode) + { + //Given an API call + //When asking for current temp and no zip code is given + //Then returns a 400 + // Arrange + var (weatherController, getWeatherHttpClient) = Factory(); + // Act + var result = await weatherController.CurrentTemp(zipCode); + + // Assert + Assert.AreEqual(400, (result.Result as BadRequestObjectResult).StatusCode); + } + */ + /* + // this shows how to setup the stub and fake the return from weather provider [TestMethod] - public async Task WeatherController_GetCurrentTemp_ZipCode_CallsWithZipCode() + public async Task WeatherController_CurrentTemp_ValidZipCode_CallsWithZipCode() { - /** - * Given an API call - * When asking for current temp - * Then it calls the weather Api with the correct zip code - */ + // Given an API call + // When asking for current temp + // Then it calls the weather Api with the correct zip code + // Arrange var zipCode = 57105; var (weatherController, getWeatherHttpClient) = Factory(); - // fake the return from weather provider with MOQ + // setup the stub and fake the return from weather provider with MOQ var fakeTemp = 72.6; getWeatherHttpClient.Setup(wp => wp.GetCurrentTempAsync(zipCode)) .ReturnsAsync(fakeTemp); @@ -54,10 +112,36 @@ public async Task WeatherController_GetCurrentTemp_ZipCode_CallsWithZipCode() // Assert getWeatherHttpClient.Verify(w => w.GetCurrentTempAsync(zipCode), Times.Once); - // Note this test may not be the most useful, but it helps move us forward with TDD + // Note: this test may not be the most useful, but it helps move us forward with TDD // some may just delete this test or skip writting it - } + } + */ + /* + // test the happy path + [TestMethod] + public async Task WeatherController_CurrentTemp_ValidZipCode_ReturnsTheTemp() + { + // first make/leave WeatherController return zipcode in result + // Arrange + var zipCode = 57105; + var (weatherController, getWeatherHttpClient) = Factory(); + var fakeTemp = 72.6; + getWeatherHttpClient.Setup(wp => wp.GetCurrentTempAsync(zipCode)) + .ReturnsAsync(fakeTemp); + // Act + var result = await weatherController.CurrentTemp(zipCode); + + // Assert + Assert.AreEqual(200, (result.Result as OkObjectResult).StatusCode); + Assert.AreEqual(fakeTemp, (result.Result as OkObjectResult).Value); + } + */ + // more tests could explore exception handling + + #endregion + #region completed tests + /* [TestMethod] public async Task WeatherController_GetTemp_NoZipCode_Returns400() { @@ -102,11 +186,11 @@ public async Task WeatherController_GetPastTemp_NoDateTimeOrInvalid_Returns400(s [TestMethod] public async Task WeatherController_GetPastTemp_ZipCodeAndDate_CallsProviderWithZipCodeAndDate() { - /** - * Given an API call - * When asking for past temp - * Then it calls the weather Api with the correct zip code and date time - */ + // + // Given an API call + // When asking for past temp + // Then it calls the weather Api with the correct zip code and date time + // // Arrange var zipCode = 57105; var date = DateTime.Now; @@ -149,6 +233,8 @@ public async Task WeatherController_GetPastTemp_ZipCodeAndDate_CallsProviderWith //Assert.AreEqual(fakeResponse.Current.TempF, weatherResult.Current.TempF); } - // TODO add more tests and code if formatting the data response to abstract from Apiux is required + */ + #endregion + //// TODO add more tests and code if formatting the data response to abstract from Apiux is required } } diff --git a/source/api/Exceptions/NotFoundException.cs b/source/api/Exceptions/NotFoundException.cs new file mode 100644 index 0000000..d126581 --- /dev/null +++ b/source/api/Exceptions/NotFoundException.cs @@ -0,0 +1,15 @@ +using System; + +namespace Api.Weather.Exceptions +{ + /// + /// Custom exception for something that wasn't found + /// + [Serializable] + public class NotFoundException : Exception + { + public NotFoundException(string message) : base(message) + { + } + } +} \ No newline at end of file diff --git a/source/api/Weather/ApiuxWeatherForecastResponse.cs b/source/api/Weather/ApiuxWeatherForecastResponse.cs index 12d681d..890b14d 100644 --- a/source/api/Weather/ApiuxWeatherForecastResponse.cs +++ b/source/api/Weather/ApiuxWeatherForecastResponse.cs @@ -1,8 +1,6 @@ using Newtonsoft.Json; using System; using System.Collections.Generic; -using System.Linq; -using System.Threading.Tasks; namespace Api.Weather { diff --git a/source/api/Weather/ApixuClient.cs b/source/api/Weather/ApixuClient.cs index 6cd1ed6..2614967 100644 --- a/source/api/Weather/ApixuClient.cs +++ b/source/api/Weather/ApixuClient.cs @@ -1,4 +1,5 @@ -using System; +using Api.Weather.Exceptions; +using System; using System.Net.Http; using System.Net.Http.Headers; using System.Threading.Tasks; @@ -27,7 +28,19 @@ public ApixuClient(HttpClient httpClient) public async Task GetCurrentTempAsync(int zipCode) { - var response = await httpClient.GetStringAsync($"current.json?key={apiKey}&q={zipCode}"); + // suggestion is to make zipCode into a string and make it postal code instead + string response = string.Empty; + //try + //{ + response = await httpClient.GetStringAsync($"current.json?key={apiKey}&q={zipCode}"); + //} + //catch(HttpRequestException) + //{ + // for ApixuClientTests_GetTemp_GivenAZipCode_NotFound_ThrowsException discussion + // // TODO check the body of the exception + // throw new NotFoundException($"{zipCode} didn't work"); + //} + var weather = ApiuxWeatherCurrentResponse.FromJson(response); return weather.Current.TempF; } diff --git a/source/api/Weather/WeatherController.cs b/source/api/Weather/WeatherController.cs index ec888fa..72e5801 100644 --- a/source/api/Weather/WeatherController.cs +++ b/source/api/Weather/WeatherController.cs @@ -15,33 +15,44 @@ public WeatherController(IGetWeatherHttpClient weatherHttpClient) } [HttpGet("[action]")] - public async Task> CurrentTemp([FromQuery(Name = "zipcode")] int zipCode) + public async Task> CurrentTemp([FromQuery] int zipCode) { - if (zipCode == 0) - { - return BadRequest($"{nameof(zipCode)} cannot be 0"); - } + // starting code + //if (zipCode == 0) + //{ + // return BadRequest($"{nameof(zipCode)} is not valid"); + //} - var result = await weatherHttpClient.GetCurrentTempAsync(zipCode); - return Ok(result); - } + return Ok(zipCode); - [HttpGet("[action]")] - public async Task> PastWeather([FromQuery(Name = "zipcode")] int zipCode, [FromQuery(Name = "dateTime")] string dateTime) - { - if(zipCode == 0) - { - return BadRequest($"{nameof(zipCode)} cannot be 0"); - } - - if (string.IsNullOrWhiteSpace(dateTime) || !DateTime.TryParse(dateTime, out var parsedDateTime)) - { - return BadRequest($"{nameof(dateTime)} must be a valid date"); - } - - var weather = await weatherHttpClient.GetPastWeatherAsync(zipCode, parsedDateTime); - return weather; + + // final product + // http://structnet.com/instructions/zip_min_max_by_state.html + //if (zipCode <= 1000 || zipCode > 99999) + //{ + // return BadRequest($"{nameof(zipCode)} cannot be 0"); + //} + + //var result = await weatherHttpClient.GetCurrentTempAsync(zipCode); + //return Ok(result); } + //[HttpGet("[action]")] + //public async Task> PastWeather([FromQuery(Name = "zipcode")] int zipCode, [FromQuery(Name = "dateTime")] string dateTime) + //{ + // if(zipCode == 0) + // { + // return BadRequest($"{nameof(zipCode)} cannot be 0"); + // } + + // if (string.IsNullOrWhiteSpace(dateTime) || !DateTime.TryParse(dateTime, out var parsedDateTime)) + // { + // return BadRequest($"{nameof(dateTime)} must be a valid date"); + // } + + // var weather = await weatherHttpClient.GetPastWeatherAsync(zipCode, parsedDateTime); + // return weather; + //} + } }