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

Assignment4 #87

Closed

Conversation

@jacob-berger
Copy link

jacob-berger commented Feb 4, 2020

Still need to add tests for UserController and GroupController

Still working on adding UserControllerTests and GroupControllerTests
@kenny2892

This comment has been minimized.

Copy link

kenny2892 commented Feb 6, 2020

I will review this.

Copy link

kenny2892 left a comment

Assignment 4

  • WebAPI Analyzers enabled ✔
  • DI properly configured
    • Services properly registered with correct scopes ✔
    • DB context properly registered ✔
    • AutoMapper properly registered ✔
  • NSwag properly setup ✔
  • API controllers properly expose CRUD operations ✔
  • Controllers are all unit tested
    You're missing unit tests for the User and Group Controllers.

Extra Credit

  • Unit tests in EntityServiceTests fixed approprietly
  • Mocking framework used to create the test doubles ✔

ActionResult<Gift> result = await giftController.Get(12);

Assert.IsInstanceOfType(result.Result, typeof(OkObjectResult));

This comment has been minimized.

Copy link
@kenny2892

kenny2892 Feb 6, 2020

You should also check the returned gift to make sure it is the correct gift.


ActionResult<Gift> result = await giftController.Put(12, gift);

Assert.IsNotNull(result);

This comment has been minimized.

Copy link
@kenny2892

kenny2892 Feb 6, 2020

You should also check that the result is an OkObjectResult and that the result had the updated gift.

Copy link
Collaborator

Keboo left a comment

(jacob-berger)

Assignment4

Project Analysis Summary
Nullable Enabled:
Unexpected Disabled Warnings:
Appropriately handle all warnings:
ProjectName Nullable Solutions Analyzers Unexpected Disabled Warnings Waning Level
SecretSanta.Api Enable SecretSanta.sln IntelliTect Analyzers, Microsoft CodeQuality Analyzers, Microsoft .NET Core Analyzers, Microsoft .NET Framework Analyzers, Microsoft.CodeAnalysis.Analyzers, Microsoft.CodeAnalysis.CSharp.Analyzers, Microsoft.CodeAnalysis.VersionCheckAnalyzer, Microsoft.EntityFrameworkCore.Analyzers 4
SecretSanta.Business Enable SecretSanta.sln IntelliTect Analyzers, Microsoft CodeQuality Analyzers, Microsoft .NET Core Analyzers, Microsoft .NET Framework Analyzers, Microsoft.CodeAnalysis.VersionCheckAnalyzer, Microsoft.EntityFrameworkCore.Analyzers 4
SecretSanta.Data Enable SecretSanta.sln IntelliTect Analyzers, Microsoft CodeQuality Analyzers, Microsoft .NET Core Analyzers, Microsoft .NET Framework Analyzers, Microsoft.CodeAnalysis.VersionCheckAnalyzer, Microsoft.EntityFrameworkCore.Analyzers 4
SecretSanta.Web Enable SecretSanta.sln IntelliTect Analyzers, Microsoft CodeQuality Analyzers, Microsoft .NET Core Analyzers, Microsoft .NET Framework Analyzers, Microsoft.CodeAnalysis.VersionCheckAnalyzer 4
SecretSanta.Api.Tests Enable SecretSanta.sln IntelliTect Analyzers, Microsoft CodeQuality Analyzers, Microsoft .NET Core Analyzers, Microsoft .NET Framework Analyzers, Microsoft.CodeAnalysis.Analyzers, Microsoft.CodeAnalysis.CSharp.Analyzers, Microsoft.CodeAnalysis.VersionCheckAnalyzer, Microsoft.EntityFrameworkCore.Analyzers 4
SecretSanta.Business.Tests Enable SecretSanta.sln IntelliTect Analyzers, Microsoft CodeQuality Analyzers, Microsoft .NET Core Analyzers, Microsoft .NET Framework Analyzers, Microsoft.CodeAnalysis.VersionCheckAnalyzer, Microsoft.EntityFrameworkCore.Analyzers 4
SecretSanta.Data.Tests Enable SecretSanta.sln IntelliTect Analyzers, Microsoft CodeQuality Analyzers, Microsoft .NET Core Analyzers, Microsoft .NET Framework Analyzers, Microsoft.CodeAnalysis.VersionCheckAnalyzer, Microsoft.EntityFrameworkCore.Analyzers 4
SecretSanta.Web.Tests Enable SecretSanta.sln IntelliTect Analyzers, Microsoft CodeQuality Analyzers, Microsoft .NET Core Analyzers, Microsoft .NET Framework Analyzers, Microsoft.CodeAnalysis.VersionCheckAnalyzer 4

Main Assignment

  • WebAPI Analyzers enabled ✔
  • DI properly configured ✔
  • Services properly registered with correct scopes ✔
  • DB context properly registered ✔
  • AutoMapper properly registered ✔
  • NSwag properly setup ✔
  • API controllers properly expose CRUD operations ✔
  • Controllers are all unit tested - Not all of the controllers are tested. This was also caught in the peer review.

Extra Credit

  • Unit tests in EntityServiceTests fixed appropriately
  • Mocking framework used to create the test doubles ✔

Peer Review

  • On time
  • Accurate
  • Appropriate comments given
public async Task Delete_Success()
{
var service = new Mock<IGiftService>();
var gift = SampleData.CreateN64Gift();

This comment has been minimized.

Copy link
@Keboo

Keboo Feb 11, 2020

Collaborator

Local variable 'gift' should be used

@@ -2,6 +2,7 @@
<PropertyGroup>
<TargetFramework>netcoreapp3.1</TargetFramework>
<Nullable>enable</Nullable>
<IncludeOpenAPIAnalyzers>true</IncludeOpenAPIAnalyzers>

This comment has been minimized.

Copy link
@Keboo

Keboo Feb 11, 2020

Collaborator

This certainly woks, but outside of web projects it does not add any value

[ProducesResponseType(StatusCodes.Status200OK)]
public async Task<IActionResult> Delete(int id)
{
if (await GiftService.DeleteAsync(id) == true)

This comment has been minimized.

Copy link
@Keboo

Keboo Feb 11, 2020

Collaborator
Suggested change
if (await GiftService.DeleteAsync(id) == true)
if (await GiftService.DeleteAsync(id))
@Keboo Keboo closed this Feb 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.