Ease builds on CI and locally #364
Conversation
- react to a smallish breaking change in newer Owin packages - gather more information when restoring RuntimePortable.sln - use `$(RestoreUseSkipNonexistentTargets)` as well - ignore binary logs - ensure the .NET Core 2.1 VS component is installed - allow use of any 2.1.5xx .NET SDK - add a few words to code analysis dictionary - exclude files generated for `net45` legacy projects don't cause problems when compiling for `netstandard2.0` - handle additional `ReflectionTypeLoadException`s when using the xUnit `msbuild` runner
|
Building on TeamCity for validation |
| @@ -429,7 +429,7 @@ public async Task Invoke_BuildsUriWithHostAndPort() | |||
| [InlineData(@"-_.~+""<>^`{|}")] | |||
| // random unicode characters | |||
| [InlineData("激光這")] | |||
| [InlineData("%24")] | |||
| [InlineData("%24", Skip="Decoded to '$' at the moment. !!! Investigate !!!")] | |||
There was a problem hiding this comment.
@Tratcher is this related to a known breaking change in the underlying Owin packages❔
There was a problem hiding this comment.
I don't think so. What versions did you update from and to?
'$' is not a reserved character in request paths, it's supposed to be un-escaped.
There was a problem hiding this comment.
We went from Microsoft.Owin v2.0.2 to v4.2.2
There was a problem hiding this comment.
Maybe aspnet/AspNetKatana@7db01eb, but that was 4.0.0, years ago.
There was a problem hiding this comment.
That makes sense given how aged our dependency was. I'll create a Invoke_CreatesUri_ContainingCorrectlyDecodedStrings(string encoded, string decoded) alternative to this test. Will include [InlineData("%24", "$")]. Do you have a suggestion or two for additional encodings that should be decoded when presented to the application❔
There was a problem hiding this comment.
https://www.rfc-editor.org/rfc/rfc3986#appendix-A
pchar = unreserved / pct-encoded / sub-delims / ":" / "@"
unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
sub-delims = "!" / "$" / "&" / "'" / "(" / ")"
/ "*" / "+" / "," / ";" / "="
There was a problem hiding this comment.
Yeah, I saw that BNF and realized the comments w/ the [TheoryData(...)] above are pretty off-base. Decided to just test percent encodings of $, (, ), [, ], {, and } rather than recategorizing things here.
nits: - capitalize "MSBuild" consistently - reduce indentation slightly
|
This PR is sufficient for successful WSR builds on TeamCity; should be enough for successful builds on AzDO (w/ VS 2017) as well. I need to make TFS changes (related to recent Microsoft.Owin version bumps in this repo) to make TC builds entirely successful. |
react to a smallish breaking change in newer Owin packages - add new test of `%` decoding to URI - do not restore RuntimePortable.sln directly - NuGet.exe is no longer happy w/ that solution - ignore binary logs - ensure the .NET Core 2.1 VS component is installed - allow use of any 2.1.5xx .NET SDK - add a few words to code analysis dictionary - exclude files generated for `net45` legacy projects don't cause problems when compiling for `netstandard2.0` - handle additional `ReflectionTypeLoadException`s when using the xUnit `msbuild` runner nits: - capitalize "MSBuild" consistently - reduce indentation slightly
$(RestoreUseSkipNonexistentTargets)as wellnet45legacy projects don't cause problems when compiling fornetstandard2.0ReflectionTypeLoadExceptions when using the xUnitmsbuildrunner