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

Fix for issue #118 #121

Merged
merged 3 commits into from
Apr 15, 2018
Merged

Fix for issue #118 #121

merged 3 commits into from
Apr 15, 2018

Conversation

raghavendrabankapur
Copy link
Collaborator

Including the code to accept BodyAsFile in the response when the mappings are placed in the file.

@raghavendrabankapur
Copy link
Collaborator Author

@StefH I have included the code for the fix #118 . I have run the tests too and they are all working fine.

@codecov
Copy link

codecov bot commented Apr 10, 2018

Codecov Report

Merging #121 into master will increase coverage by 0.43%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #121      +/-   ##
==========================================
+ Coverage   66.56%   66.99%   +0.43%     
==========================================
  Files          79       79              
  Lines        2862     2866       +4     
  Branches      389      390       +1     
==========================================
+ Hits         1905     1920      +15     
+ Misses        742      728      -14     
- Partials      215      218       +3
Impacted Files Coverage Δ
src/WireMock.Net/Server/FluentMockServer.Admin.cs 39.96% <75%> (+0.43%) ⬆️
src/WireMock.Net/ResponseBuilders/Response.cs 91.22% <0%> (+6.43%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a293631...507237d. Read the comment docs.

@raghavendrabankapur
Copy link
Collaborator Author

@StefH should I include any tests for this or this is fine?

@StefH
Copy link
Collaborator

StefH commented Apr 10, 2018

If you can create a simple small test, to make sure the code coverage increases, that would be nice.

@raghavendrabankapur
Copy link
Collaborator Author

@StefH I have added a test to cover the code. It required to fix one test too.


string responseBodyFilePath = Path.Combine(GetCurrentFolder(), "ResponseBodyFiles", "responsebody.json");

dynamic jsonObj = Newtonsoft.Json.JsonConvert.DeserializeObject(json);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Newtonsoft.Json is not needed here. Please import this namespace in this file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have moved this to using block and updated the commit. @StefH

@raghavendrabankapur
Copy link
Collaborator Author

@StefH updated the pull request.

@raghavendrabankapur
Copy link
Collaborator Author

@StefH please let me know if I can merge pull request?

@StefH StefH added the bug label Apr 12, 2018
@StefH
Copy link
Collaborator

StefH commented Apr 12, 2018

I will merge this PR in the weekend. And create new NuGet.

@raghavendrabankapur
Copy link
Collaborator Author

@StefH please let me know if I can merge pull request?

@StefH StefH merged commit 0abe12f into WireMock-Net:master Apr 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants