-
-
Notifications
You must be signed in to change notification settings - Fork 213
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
Support Microsoft.AspNetCore for net 4.6.1 and up #185
Conversation
Codecov Report
@@ Coverage Diff @@
## master #185 +/- ##
==========================================
+ Coverage 71.19% 71.72% +0.52%
==========================================
Files 89 89
Lines 3236 3240 +4
Branches 425 423 -2
==========================================
+ Hits 2304 2324 +20
+ Misses 763 750 -13
+ Partials 169 166 -3
Continue to review full report at Codecov.
|
@@ -39,7 +39,7 @@ public AspNetCoreSelfHost([NotNull] WireMockMiddlewareOptions options, [NotNull] | |||
|
|||
_logger = options.Logger ?? new WireMockConsoleLogger(); | |||
|
|||
foreach (string uriPrefix in uriPrefixes) | |||
foreach (string uriPrefix in uriPrefixes) // .Select(u => u.EndsWith("/") ? u : $"{u}/")) // Always append an / at the end. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the code supposed to be commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes should be removed.
@@ -26,7 +27,7 @@ public OwinSelfHost([NotNull] WireMockMiddlewareOptions options, [NotNull] param | |||
|
|||
_logger = options.Logger ?? new WireMockConsoleLogger(); | |||
|
|||
foreach (string uriPrefix in uriPrefixes) | |||
foreach (string uriPrefix in uriPrefixes) // .Select(u => u.EndsWith("/") ? u : $"{u}/")) // Always append an / at the end. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the code supposed to be commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes should be removed.
@@ -14,6 +14,36 @@ public class RequestWithPathTests | |||
{ | |||
private const string ClientIp = "::1"; | |||
|
|||
// [Fact] : TODO : this test fails??? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably remove comment and TODO?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I keep this for now as reminder,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the test doesn't fail anymore right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow this test still fails. Also for netcore.
So for now i've removed the test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well then the underlying problem (that WireMock.Net does not handle encoded URLs correctly) still persists :(.
@chrischu Thanks for your review. I've updated some code and commented on your comments. Can you please approve? |
Since it is already merged: Is there a plan for creating a new release with these changes? |
I'll create a new version today. |
1.0.4.11 NuGet created. Please test. |
No description provided.