TextReader/Writer Span overloads and tests #23786

WinCPP · 2017-09-04T17:23:35Z

Appreciate your review feedback on the changes in this PR.

In this PR,

New Span / ReadonlySpan overloads as mentioned in #22406. The buffer based overloads will be done when buffer is available.
Added TextReader and TextWriter tests as per comment here.

Contributes to #22406.

Thanks,
Mandar

WinCPP · 2017-09-04T18:44:41Z

@stephentoub NETFX, UWP CoreCLR and WP NETNative are failing on Span type arguments. Is there something that I need to additionally modify...?

stephentoub · 2017-09-05T01:36:23Z

src/System.IO/tests/System.IO.Tests.csproj

+    <Compile Include="TextReader\TextReaderImpl.cs" />
+    <Compile Include="TextReader\TextReaderTests.cs" />
+    <Compile Include="TextWriter\TextWriterImpl.cs" />
+    <Compile Include="TextWriter\TextWriterTests.cs" />


The tests for the new APIs should be separated out from tests for the existing ones, such that the file containing the new ones can be conditioned to only build when those APIs exist, e.g. Condition="'$(TargetGroup)' == 'netcoreapp'". That'll help with the build failures.

WinCPP · 2017-09-05T03:12:24Z

@dotnet-bot test NETFX x86 Release Build please

WinCPP · 2017-09-05T05:10:49Z

@stephentoub StackFrame tests are failing on NETFX x86 build. I'm not sure what to do... Following are the errors. Please help.

    Starting:    System.Diagnostics.StackTrace.Tests
      System.Diagnostics.Tests.StackFrameTests.Ctor_FNeedFileInfo(fNeedFileInfo: True) [FAIL]
        Assert.Equal() Failure
        Expected: 0
        Actual:   7
        Stack Trace:
          D:\j\workspace\windows-TGrou---2a8f9c29\src\System.Diagnostics.StackTrace\tests\StackFrameTests.cs(166,0): at System.Diagnostics.Tests.StackFrameTests.VerifyStackFrameSkipFrames(StackFrame stackFrame, Boolean isFileConstructor, Int32 skipFrames, MethodInfo expectedMethod, Boolean isCurrentFrame)
          D:\j\workspace\windows-TGrou---2a8f9c29\src\System.Diagnostics.StackTrace\tests\StackFrameTests.cs(34,0): at System.Diagnostics.Tests.StackFrameTests.Ctor_FNeedFileInfo(Boolean fNeedFileInfo)
      System.Diagnostics.Tests.StackFrameTests.Ctor_FNeedFileInfo(fNeedFileInfo: False) [FAIL]
        Assert.Equal() Failure
        Expected: 0
        Actual:   7
        Stack Trace:
          D:\j\workspace\windows-TGrou---2a8f9c29\src\System.Diagnostics.StackTrace\tests\StackFrameTests.cs(166,0): at System.Diagnostics.Tests.StackFrameTests.VerifyStackFrameSkipFrames(StackFrame stackFrame, Boolean isFileConstructor, Int32 skipFrames, MethodInfo expectedMethod, Boolean isCurrentFrame)
          D:\j\workspace\windows-TGrou---2a8f9c29\src\System.Diagnostics.StackTrace\tests\StackFrameTests.cs(34,0): at System.Diagnostics.Tests.StackFrameTests.Ctor_FNeedFileInfo(Boolean fNeedFileInfo)
      System.Diagnostics.Tests.StackFrameTests.Ctor_SkipFrames_FNeedFileInfo(skipFrames: 0, fNeedFileInfo: True) [FAIL]
        Assert.Equal() Failure
        Expected: 0
        Actual:   8
        Stack Trace:
          D:\j\workspace\windows-TGrou---2a8f9c29\src\System.Diagnostics.StackTrace\tests\StackFrameTests.cs(166,0): at System.Diagnostics.Tests.StackFrameTests.VerifyStackFrameSkipFrames(StackFrame stackFrame, Boolean isFileConstructor, Int32 skipFrames, MethodInfo expectedMethod, Boolean isCurrentFrame)
          D:\j\workspace\windows-TGrou---2a8f9c29\src\System.Diagnostics.StackTrace\tests\StackFrameTests.cs(57,0): at System.Diagnostics.Tests.StackFrameTests.Ctor_SkipFrames_FNeedFileInfo(Int32 skipFrames, Boolean fNeedFileInfo)
      System.Diagnostics.Tests.StackFrameTests.Ctor_SkipFrames_FNeedFileInfo(skipFrames: 0, fNeedFileInfo: False) [FAIL]
        Assert.Equal() Failure
        Expected: 0
        Actual:   8
        Stack Trace:
          D:\j\workspace\windows-TGrou---2a8f9c29\src\System.Diagnostics.StackTrace\tests\StackFrameTests.cs(166,0): at System.Diagnostics.Tests.StackFrameTests.VerifyStackFrameSkipFrames(StackFrame stackFrame, Boolean isFileConstructor, Int32 skipFrames, MethodInfo expectedMethod, Boolean isCurrentFrame)
          D:\j\workspace\windows-TGrou---2a8f9c29\src\System.Diagnostics.StackTrace\tests\StackFrameTests.cs(57,0): at System.Diagnostics.Tests.StackFrameTests.Ctor_SkipFrames_FNeedFileInfo(Int32 skipFrames, Boolean fNeedFileInfo)
    Finished:    System.Diagnostics.StackTrace.Tests

hughbe · 2017-09-05T08:31:34Z

src/System.IO/tests/TextReader/TextReaderTests.cs

+        public void ArgumentNullOnNullArray()
+        {
+            var baseInfo = GetCharArray();
+            var tr = baseInfo.Item2;


C# 7 tuples fcould make this clearer

Done. Please check.

Looks great thanks

WinCPP · 2017-09-05T10:56:45Z

@dotnet-bot test Windows x64 Debug Build please

stephentoub · 2017-09-05T14:34:51Z

NETFX, UWP CoreCLR and WP NETNative are failing on Span type arguments. Is there something that I need to additionally modify...?

This isn't specific to your PR... something's causing these tests to fail across the board.

WinCPP · 2017-09-05T15:33:07Z

@dotnet-bot test Packaging All Configurations x64 Debug Build please

stephentoub · 2017-09-05T15:42:05Z

src/System.Runtime.Extensions/src/System/IO/TextReader.cs

-                buffer[index + n++] = (char)ch;
-            } while (n < count);
+                destination[n++] = (char)ch;
+            } while (n < destination.Length);


This loop looks like it'll hit an IndexOutOfRangeException if destination.Length == 0. Is there a test for that? In fact, on netfx this code throws an IndexOutOfRangeException:

using System.IO; class Program { static void Main() { var reader = new MyTextReader(); reader.Read(new char[0], 0, 0); } } class MyTextReader : TextReader { public override int Read() => 'a'; }

cc: @JeremyKuhne

I'd prefer seeing the loop written like:

int n; for (n = 0; n < destination.Length; n++) { int ch = Read(); if (ch == -1) { break; } destination[n] = (char)ch; } return n;

which IMHO not only makes it easier to understand, but also addresses the out-of-bounds exception.

@stephentoub As per the above review feedback, I have to invert the logic to have Read(Span<char>) delegate to Read(char []...). Above is how it was originally in the code, do you want me to make changes along with the straightening of the logic...?

I suggest leaving the existing code for now. I'll open an issue for the problematic behavior. It can be addressed in a separate PR.

stephentoub · 2017-09-05T15:46:56Z

src/System.Runtime.Extensions/src/System/IO/TextReader.cs

+        // buffer character array starting at position
+        // index. Returns the actual number of characters read.
+        //
+        public virtual int Read(Span<char> destination)


This approach needs to be inverted. We're adding a virtual method here, which means existing derivations of TextReader may have overridden Read(char[], ...) but couldn't yet have overridden Read(Span<char>). If new code then calls Read(Span<char>) instead of Read(char[], ...), it won't get the derived behavior but rather than potentially-incorrect/potentially-inefficient base implementation. So the base Read(Span<char>) needs to delegate to the base Read(byte[], ...) rather than the other way around... you can see examples of this in the new Stream.Read/Write span-based methods in coreclr.

Same issue applies other base virtuals being added.

stephentoub · 2017-09-05T15:48:15Z

src/System.IO/tests/TextReader/TextReaderImpl.cs

+
+namespace System.IO.Tests
+{
+    public class TextReaderImpl : TextReader


Rather than TextReaderImpl, it'd be better to be descriptive about what this is, e.g. CharArrayTextReader, or something like that.

Changed to CharArrayTextReader...

stephentoub · 2017-09-05T15:48:21Z

src/System.IO/tests/TextReader/TextReaderImpl.cs

+{
+    public class TextReaderImpl : TextReader
+    {
+        private char[] _charBuffer;


Nit: readonly

stephentoub · 2017-09-05T15:48:26Z

src/System.IO/tests/TextReader/TextReaderImpl.cs

+    public class TextReaderImpl : TextReader
+    {
+        private char[] _charBuffer;
+        int _charPos = 0;


Nit: private

stephentoub · 2017-09-05T15:50:01Z

src/System.IO/tests/TextWriter/TextWriterImpl.cs

+
+namespace System.IO.Tests
+{
+    public class TextWriterImpl : TextWriter


Same naming comment.

Done... changed to CharArrayTextWriter

stephentoub · 2017-09-05T15:50:36Z

src/System.IO/tests/TextWriter/TextWriterTests.cs

+        protected static object firstObject = (object)1;
+        protected static object secondObject = (object)"[second object]";
+        protected static object thirdObject = (object)"<third object>";
+        protected static object[] multipleObjects = new object[] { firstObject, secondObject, thirdObject };


Nit: all of these statics should begin with a capital letter.

stephentoub

Thanks for working on these.

danmoseley · 2017-09-05T16:56:45Z

src/System.IO/tests/TextWriter/TextWriterTests.cs

+        protected static object thirdObject = (object)"<third object>";
+        protected static object[] multipleObjects = new object[] { firstObject, secondObject, thirdObject };
+
+


nit extra blank line

danmoseley · 2017-09-05T16:57:27Z

src/System.IO/tests/TextWriter/TextWriterTests.cs

+        {
+            return new char[]{
+                char.MinValue
+                ,char.MaxValue


what's with the commas? we prefer them at the end of the line.

hmmm. I copied this block from here and continued to follow the same style as per the existing code...

Do you want me to change in both the places...?

done just in this place :) ....

if you could fix both that would be great.

done in 4 places... through out the solution...

danmoseley · 2017-09-05T16:59:17Z

src/System.IO/tests/TextWriter/TextWriterTests.cs

+        [Fact]
+        public void WriteLineDoubleTest()
+        {
+            using (TextWriterImpl tw = GetTextWriter())


You could have a utility function like
void AssertWriteLine(object value, string expected)
and call it like
AssertWriteLine(double.MinValue, double.MinValue.ToString());
and it would take care of creating the text writer, writing, clearing, and the newline. Making these tests 3 lines each

Not sure... but will it end up invoking the TextWriter::WriteLine(object) overload instead of exercising the individual overloads?

Ah - you're quite right. Disregard.

danmoseley · 2017-09-07T21:59:36Z

src/System.IO/tests/TextReader/TextReaderTests.cs

+        public void TestRead()
+        {
+            var baseInfo = GetCharArray();
+            var tr = baseInfo.textReader;


using (var tr = baseInfo.textReader) ?

If baseInfo is still holding a reference to textReader will using (var tr = baseInfo.textReader) make a difference?

Scratched my head a lot on this when I looked at existing test cases for reference which were not doing the same. Please refer here

using statement just causes Dispose(), it doesn't matter if anyone else holds a reference, so long as they don't use it. the code could be written

var baseInfo; var tr = null; try { baseInfo = GetCharArray(); tr = baseInfo.textReader; .... finally { if (tr != null) tr.Dispose(); }

that is what I would do in product. The using syntax doesn't isn't flexible enough to let you to wrap the GetCharArray() also, but it really doesn't matter for a test. Actually I think it's fine as you've written it, given it's a test.

Thanks got it. I am not using the additional reference later on anyways. So doesn't make any difference even if it is invalidated. Between, something wrong with github. Didn't get any notification for your new comments on email and in between I just went ahead and made all the changes!!! So please re-review.

Thanks,
Mandar

mmitche · 2017-09-07T22:05:07Z

GitHub is having delays delivering Webhook payloads: https://status.github.com/. They appear to be an hour and 10 minutes or so behind at the moment.

14:43 Pacific Daylight TimeWebhook deliveries are delayed while we process a large backlog of events.

Matt

danmoseley · 2017-09-07T22:19:31Z

src/System.IO/tests/TextReader/TextReaderTests.cs

+        public async void ReadBlockAsyncCharArr()
+        {
+            var baseInfo = GetCharArray();
+            var tr = baseInfo.textReader;


these tr's should ideally be in a using statement.

as above, it's not important.

danmoseley

@stephentoub or @JeremyKuhne or @pjanotti should sign off

danmoseley · 2017-09-08T16:02:25Z

MembersMustExist : Member 'System.Random.NextBytes(System.Span<System.Byte>)' does not exist in the implementation but it does exist in the contract. ...

was the build in a broken state for a period? does not appear to be now.

@dotnet-bot test this please

WinCPP · 2017-09-08T17:00:08Z

@danmosemsft I have implemented the review comment to use using construct and have made some of the strings into static readonly as they don't need to be created again and again. Appreciate your feedback.

WinCPP · 2017-09-08T17:02:18Z

src/System.Runtime.Extensions/src/Resources/Strings.resx

@@ -1,5 +1,64 @@
 <?xml version="1.0" encoding="utf-8"?>
 <root>
+  <!-- 


Note that for some reasons VS 2017 Community Edition has added spaces in some places when I added a new error message.

WinCPP · 2017-09-08T17:48:02Z

@dotnet-bot test OSX x64 Debug Build please

mmitche · 2017-09-08T17:48:54Z

I see this in the log:

10:31:06 /Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/dir.traversal.targets(81,3): warning MSB4011: "/Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/Tools/Dumpling.targets" cannot be imported again. It was already imported at "/Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/Tools/tests.targets (541,3)". This is most likely a build authoring error. This subsequent import will be ignored. [/Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/src/tests.builds]
10:31:06 /Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/Tools/tests.targets(475,5): warning :    System.Net.Ping.Functional.Tests  Total: 31, Errors: 0, Failed: 1, Skipped: 0, Time: 1.467s [/Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/src/System.Net.Ping/tests/FunctionalTests/System.Net.Ping.Functional.Tests.csproj]
10:31:06 /Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/Tools/tests.targets(475,5): warning MSB3073: The command "/Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/bin/tests/System.Net.Ping.Functional.Tests/netcoreapp-OSX-Debug-x64/RunTests.sh /Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/bin/testhost/netcoreapp-OSX-Debug-x64/" exited with code 1. [/Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/src/System.Net.Ping/tests/FunctionalTests/System.Net.Ping.Functional.Tests.csproj]
10:31:06 /Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/Tools/tests.targets(483,5): error : One or more tests failed while running tests from 'System.Net.Ping.Functional.Tests' please check /Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/bin/tests/System.Net.Ping.Functional.Tests/netcoreapp-OSX-Debug-x64/testResults.xml for details! [/Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/src/System.Net.Ping/tests/FunctionalTests/System.Net.Ping.Functional.Tests.csproj]
10:31:06 /Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/dir.traversal.targets(77,5): error : (No message specified) [/Users/dotnet-bot/j/workspace/dotnet_corefx/master/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/src/tests.bui
```lds]

WinCPP · 2017-09-08T18:02:10Z

@mmitche yup I saw that, but since it is not related to the changes that I'm doing, I just thought I would ask dotnet bot to retest the specific platform.

WinCPP · 2017-09-08T19:05:02Z

@dotnet-bot test Windows x86 Release Build please

WinCPP · 2017-09-08T20:47:54Z

@stephentoub @danmosemsft I have squashed the commits into single one. Kindly let me know your review feedback.

To work on #22408 #22409, etc. I need the commits from this PR and managing merges within my fork is becoming a bit messy if I change anything on this PR... so if we could proceed on this one, it will help me. Thanks!

WinCPP · 2017-09-09T11:31:48Z

@dotnet-bot test Linux x64 Release Build please

stephentoub · 2017-09-10T03:18:11Z

src/System.Runtime.Extensions/src/System/IO/TextReader.cs

+                int numRead = Read(buffer, 0, destination.Length);
+                if ((uint)numRead > destination.Length)
+                {
+                    throw new IOException(SR.IO_StreamTooLong);


There's not necessarily a Stream involved here, so the error name and message is a bit misleading. I suggest something instead like "The read operation returned an invalid length."

stephentoub · 2017-09-11T15:25:15Z

src/System.IO/tests/StringWriter/StringWriterTests.cs

+                '-',
+                ';',
+                '\u00E6'
+            };


Is there any change to these three files other than formatting? Assuming it's just formatting, please revert the changes to those files.

I asked him to fix the commas

In that case, if we're going to modify it, why not consolidate the now four copies of this?

Oh sorry. Looks like I missed this comment. Let me check...

Actually there are two sets of the test data...

One in StreamReaderTests.cs which I copied over to TextReaderTests.cs and TextWriterTests.cs and,

Other one in StreamWriter.WriteTests.cs with a copy in StringWriterTests.cs

The second set uses fewer characters than the first set. Do we want to consolidate the copies which might entail re-writing some test cases? Given that it is test data how about allowing each test to use its data? Just a thought...

Also I'm ok reverting back the formatting for the other places that I touched...

Inputs please.

@stephentoub Bumping the thread. Please see above comment.

In addition, I made changes to StreamWriter.WriteTests.cs (method setupArray) and StringWriterTests.cs (method getCharArray) to attempt common input and tests ran fine. However for actual consolidation to single copy, it would entail restructuring the five test classes and extract data generation to a common base or test data provider class, also considering the fact that there are other smaller character data arrays which may differ across the five test files.

Appreciate your inputs.

If there are two distinct sets of data, it's fine for them to remain separate. My objection is to making copies of entire sets of data, and more generally to having exact duplicates... we should put the shared set or sets into some shareable place and use those shared sets rather than copy and pasting.

@stephentoub I have tried to consolidate the data into a new TestDataProvider class. Please note that as mentioned in previous comment, even if there are two sets of data that subtly differ, I have consolidated them because with the longer of the two, the tests still passed. That was local testing. I will monitor the CI tests. Please check and let me know if the changes are acceptable.

Also I have corrected the coding style issues that VS 2017 community edition IDE pointed out to me...

stephentoub

Thanks for working on this.

KrzysztofCwalina · 2017-09-11T15:38:49Z

src/System.Runtime.Extensions/ref/System.Runtime.Extensions.cs

@@ -1512,6 +1514,7 @@ public abstract partial class TextWriter : System.MarshalByRefObject, System.IDi
        public virtual void Write(uint value) { }
        [System.CLSCompliantAttribute(false)]
        public virtual void Write(ulong value) { }
+        public virtual void Write(ReadOnlySpan<char> source) { }


since the char[] parameter is called "buffer", I think we should stick with "buffer". Same with WriteLine.

I continued with the word mentioned in the approved API. Then likewise should it be buffer for Read/Block overloads as well, instead of destination...?

If we agree that as a general guideline, then I will use the same rule for #22408, #22409, etc. as well. Appreciate your inputs.

Hi Mandar, is this the only item remaining regarding the PR? Thanks.

This one and please see the conversation just above this one over here. Thanks!!!

Yes, we should use "buffer" instead of "destination" in all APIs with "buffer" precedent.

@KrzysztofCwalina, what about Stream.Read/Write{Async}? Those use "buffer" as well, but it was in the context of those methods that we'd settled on "source" and "destination". Are you suggesting that those change, too?

OK< since we already merged some "destination" parameters similar to this one. I am ok with merging this PR and then doing cleanup if we decide it's worth it.

WinCPP · 2017-09-11T21:47:51Z

@dotnet-bot test Linux x64 Release Build please

pjanotti · 2017-09-21T17:39:31Z

Thanks for your contribution Mandar.

* Microsoft.ServiceModel.Syndication skeleton project * Adding the existing classes of SyndicationFeed from .net fx * Added the needed references to get the code to compile * Changed some namespaces * Fixed errors when reading feeds and replaced some buffers * Cleaning code for PR * Deleted some unused files * Added the posibility that the user creates his own date parser when reading a feed from outside, also as part of our default date parser, if it can't parse the date it will just assign a default date to avoid the crash * Added correct testnames and Copyright * Initial changes to add custom parsers * Added more delegates as parsers * Added custom parser delegates * Save changes * Initial SyndicationFeed * Removed the dependence of the old SR class * Cleaned the code, deleted Diagnostics, fixed throwing resources * Cleaned code from most of the unnecesary comments * Formatted the code with CodeFormatter Tool * Moved the call of itemParser to be called with each item * Test using custom parsers * Fixed image issue where image tag was using original title and url, for RSS formatting Image issue fixed and added some tests * Test clase jorge * Save changes with Jorge cass * Initial Jorge * saving changes * Save changes * Fixed image and items issue * Fixed disjoint items in atom * Run codeFormatter * Adding parsing to optional elements int the spec added unittesting * Added Icon parsing to Atom10 feedFormatter. Added unit test * Adding Async Writing RssFormater writes new optional spec elements * Added Icon writing on Arom Writer * Fixed some warnings * Improved custom parsing for RSS * Added custom parsing for Atom feed formatter, added test * added nameof() to all exceptions when needed. * Adding Extension Methods for XmlReader * Fixing code for PR * Fixing code for PR * Added check for skip days to allow only accepted days. * Improved flexibility for Default dateparser * Add wrong dates example * Fixing code review * Fixed warnings on some unawaited methods. * Added async extension methods for XmlReader * Add XmlWriterWrapper method extensions * Changed ReadCategoryFromAsync to return a SyndicationCategory * Fixed sync method exposed GetReader. * Edited XmlReaderWrapper, moved methods to extension methods. * Removed unnecesary variables from Wrappers. * Fixed bug from ServiceModel.Syndication * Make BigInteger ctor opt path for == int.MinValue reachable. The BigInteger(ReadOnlySpan<byte> value) ctor contains an optimised path for value being larger than 4 bytes and the result being equal to int.MinValue. However this path is inside a test that precludes that value, so can never be reached. Restructure so that the path is reachable. * Fix ServiceModel.SyndicationFeed project dependencies - Change M.SM.SyndicationFeed to be a .NET Standard 2.0 library - Change tests to use official .NET Core 2.0 release from preview * Add btrfs and other missing file system types (dotnet#24102) * Uppercase * Add more filesystems from stat.c. Change to stat names where possible * Add some more local friendly names * Add some more remote file types from stat.c * Add entry to switch present in enum * Build errors * Remove Fixed entries that should be RAM * comment * Change case of 0X to 0x * Move cramfs to Fixed * ConcurrentQueue 128-byte cache line (dotnet#22724) ConcurrentQueue 128-byte cache line * Add warning by default in SGEN (dotnet#24054) * Output warning by default if run the tool directly without /quiet parameter. * add quiet parameter in the command. * fix parameter error. * Update the warning. * Add the target to copy the serializer to publish folder. (dotnet#24096) * Wrap cert callback leaked exceptions in WinHttpHandler (dotnet#24107) Similar to the fix done for CurlHandler (dotnet#21938), fix WinHttpHandler so that leaked exceptions from the user-provided certificate callback are properly wrapped. The ManagedHandler still needs to be fixed since it is not wrapping properly. Contributes to #21904 * Ensure ProcessModule for main executable is first in modules list (dotnet#24106) * Disable NegotiateStreamTest fixture for distros without working Kerberos (dotnet#24098) Disable NegotiateStreamTest fixture entirely because setup-kdc.sh is broken on some distros * Expose and add tests for Guid.ctor(Span)/TryWriteBytes/TryFormat * Address remaining PR feedback * #24112 Replaced documentation summary of TryPop with text similar to TryDequeue. (dotnet#24113) * Wrap exceptions from ManagedHandler's server validation callback (dotnet#24111) To match other handlers' behaviors. * Fix libgdiplus function loading on OSX. * Prevent ServiceControllerTests from dispose when already disposed (dotnet#24042) * Disable ServiceProcessTest that has been failing on CI and official builds * Prevent dispose from been called when already disposed * Add logging to repro if RemoveService is been called twice on the same ServiceController * Data-annotations length fix (dotnet#24101) * Updated in MinLengthAttribute and MaxLengthAttribute to support ICollection<T> * Added tests * Fixed typo * Trying to address two failing checks: - Linux x64 Release Build - UWP CoreCLR x64 Debug Build * Implemented changes requested in review - Extracted Count checking to an external helper to obey DRY - Removed dependency of ICollection<T> and changed to simple reflection Count property lookup * Added requested tests * Added catch for MissingMetadataException. * Extracted code from try-catch. * Added comment as requested. * Typo correction * Remove System.Drawing.Common from the netfx compat package (temporary). * Updating corefx to use new roslyn compiler (dotnet#24076) * Updating corefx to use new roslyn compiler * Updating to new version of the compiler and use the switch so tests pass on desktop * Fix System.Reflection.Metadata tests * Stop running Math.Clamp tests on UAP as API is not there (dotnet#24125) * Stop running Math.Clamp tests on UAP as API is not there * Disable only for AOT * Fix instantiating PrincipleContext with Domain type (dotnet#24122) * Fix instantiating PrincipleContext with Domain type * Enhancement * Disable Test_Write_Metric_eventListener on UWP. (dotnet#24127) * Revert "Remove System.Drawing.Common from the netfx compat package (temporary)." This reverts commit f6b0fbd. * Update BuildTools, CoreClr, CoreFx, CoreSetup, Standard to prerelease-02019-01, preview1-25718-02, preview1-25718-03, preview1-25717-02, preview1-25718-01, respectively (dotnet#24075) * Fixed compile warning/error on FreeBSD (dotnet#24141) * Marking {ReadOnly}Span as readonly structs (dotnet#23908) * Marking {ReadOnly}Span as readonly structs, fixing issue #23809 * Adding readonly attributes to reference assemblies. * Using "readonly ref" keyword instead of attributes. * Adding a LangVersion 7.2 property * System.Drawing: Throw ArgumentNullException on Unix as well (dotnet#24140) * Throw ArgumentNullException when stream is null * Throw ArgumentNullException when stream is null * Remove invalid NameResolution tests (dotnet#24147) The Dns_GetHostEntryAsync_* are fundamentially invalid because it's possible for the broadcast address, 255.255.255.255, to have an DNS mapping via manually modifying the hosts file. This was actually happening on Mac systems as well as an virtual environment running on top of that (i.e. Windows on Parallels). Ref: https://github.com/dotnet/corefx/issues/23992#issuecomment-330250642 Contributes to #23992 * Bump system.drawing.common.testdata to 1.0.6 * Update BuildTools to prerelease-02019-02 (dotnet#24146) * Fix path to test data * Fix whitespace * Add GraphicsTests based on Mono's test suite. * Consolidate more code in the "System.Drawing" namespace. * Remove all remaining Win32 codepaths from the mono codebase. All of this code now implicitly assumes that it will be run on a Unix platform. * Consolidate the rest of the gdipFunctions.cs file into Gdip.cs and GdipNative.Unix.cs * Consolidate the GraphicsUnit and ImageType enumerations -- they were duplicated. * Remove the mono Status enum and use the Windows constants instead in all Unix code. * Move all files into the regular directory structure. Suffix them with ".Unix" and ".Windows" when there are collisions. * Tiny bit of code cleanup * Add conditionals for recent versions of mono * Remove duplicate tests. * Fix multiplying TextureBrush with a disposed matrix on Unix (dotnet#24109) * Fix multiplying TextureBrush with a disposed matrix on Unix * Enable another passing test * Fix accidentally committed file * Add an error code fixup to Bitmap.Unix.cs to match Windows. * Bump System.Drawing.Common.TestData to 1.0.6 (dotnet#24149) * Bump system.drawing.common.testdata to 1.0.6 * Fix path to test data * Fix whitespace * Cleanup - Add/simplify using statements of disposable resources (Graphics, Bitmap) * Validate HatchStyle passed to HatchBrush ctor * Delete accidentally duplicated HatchBrush tests in LinearGradientBrushTests * Remove references to historical Mono bug IDs * Renable some already passing tests * Remove Thread.Sleep workaround * Update BuildTools to prerelease-02020-01 (dotnet#24172) * Add thread-local based switch to opt-in to ManagedHandler * Address PR feedback * PR feedback * Fix memory map imports (dotnet#24176) * Fix memory map imports Imports lost the last error attribute. Add it back and change the results to be the more correct "bool". Tweak the usage based on the new return type. #24159 * Move spin wait * Remove unused FEATURE_RANDOMIZED_STRING_HASHING (dotnet#24178) * Adding System.Data.Odbc package and including in metapackage * Fix MultiplyTransform with a disposed brush * Fix for 2nd Connection hangs in Mirroring Environment (dotnet#24181) * Switch tests to use thread-local switch for ManagedHandler And as a result re-enable parallelism of the test suite, which on my machine reduces the running time of the outerloop tests from 150s to 45s. * Update CoreClr, CoreSetup, ProjectNTfs, ProjectNTfsTestILC, Standard to preview1-25720-03, preview1-25719-04, beta-25721-01, beta-25721-01, preview1-25721-01, respectively * CoreFx #22406 Span based APIs - Text Reader Writer (dotnet#23786) * [Drawing] Move remaining "Unix" files into regular directory structure * As part of this, the "Windows" version of ImageFormat.cs is now being used everywhere. This file just contains a bunch of GUID's that are not platform-specific in any way. * Change AsSpan to property Span and rename AsMemory to Memory * Add Metafile and MetaHeader tests based on Mono's System.Drawing unit tests * Update project file * MetafileTests: Remove duplicates, and re-enable tests on Unix which are now working. Delete duplicate MetaHeadertests * Rationalize using statements * Update project file * Fix Metafile exception behavior * Simplify tests, remove duplicate test * Don't catch an ArgumentException just to get a NullReferenceException instead. * Assert.False(Object.ReferenceEquals => Assert.NotSame * PR feedback * Remove duplicate tests, fix typo * Remove X11 dependency for tests which are enabled on Unix. * Workaround libgdiplus glitches Don't test metafileHeader.Bounds when using libgdiplus. Don't test metafile.GetBounds on Unix, force MetafileHeader.Bounds to return an empty rectangle when MetafileSize == 0 Don't validate graphicsUnit on Unix. * Move Syndication to root/src * Disable Build of S.SM.Syndication. * Increase file descriptor limit in S.N.Http tests on macOS * Expose/test String.Create span-based method (dotnet#23872) * Expose/test String.Create span-based method * Address PR feedback * Update build to clang/llvm 3.9 (dotnet#24177) Update scripts, docs and build pipeline docker images to clang/llvm/lldb 3.9 * Update ProjectNTfs, ProjectNTfsTestILC, Standard to beta-25722-00, beta-25722-00, preview1-25722-01, respectively (dotnet#24207) * Remove the line that will copy the generated serializer to the pack. (dotnet#24199) * Revert "Update build to clang/llvm 3.9 (dotnet#24177)" This reverts commit 21e008a. * Remove stale SetStateMachine call in test * Ssl Stream Async Write (dotnet#23715) * Change from APM to Async/Await for write side * Added nameof * Reacting to review * Reacting to review * SSLStream : Fixed spelling mistake in file name (extra a) (dotnet#24221) * Fixed spelling mistake in file name * Fix csproj * Update ProjectNTfs, ProjectNTfsTestILC to beta-25723-00, beta-25723-00, respectively (dotnet#24222)

…et/corefx#23786) Commit migrated from dotnet/corefx@c09ce65

dnfclas added the cla-already-signed label Sep 4, 2017

stephentoub reviewed Sep 5, 2017

View reviewed changes

WinCPP force-pushed the span_issue_22406 branch from 2717df1 to 63da3a8 Compare September 5, 2017 02:38

hughbe reviewed Sep 5, 2017

View reviewed changes

stephentoub reviewed Sep 5, 2017

View reviewed changes

stephentoub suggested changes Sep 5, 2017

View reviewed changes

danmoseley reviewed Sep 5, 2017

View reviewed changes

karelz added the area-System.IO label Sep 6, 2017

karelz assigned WinCPP, danmoseley and stephentoub and unassigned danmoseley Sep 6, 2017

WinCPP closed this Sep 6, 2017

WinCPP force-pushed the span_issue_22406 branch from c24190f to e150f00 Compare September 6, 2017 17:56

WinCPP reopened this Sep 6, 2017

dnfclas added the cla-already-signed label Sep 6, 2017

danmoseley reviewed Sep 7, 2017

View reviewed changes

danmoseley approved these changes Sep 8, 2017

View reviewed changes

WinCPP commented Sep 8, 2017

View reviewed changes

WinCPP force-pushed the span_issue_22406 branch from 619ab13 to a588277 Compare September 8, 2017 20:45

danmoseley requested a review from JeremyKuhne September 8, 2017 21:11

WinCPP mentioned this pull request Sep 9, 2017

CoreFx #22409 StreamWriter Span based API and tests #23915

Closed

stephentoub reviewed Sep 10, 2017

View reviewed changes

stephentoub changed the title ~~Corefx #22406 Span overloads and tests~~ TextReader/Writer Span overloads and tests Sep 11, 2017

stephentoub reviewed Sep 11, 2017

View reviewed changes

stephentoub approved these changes Sep 11, 2017

View reviewed changes

KrzysztofCwalina reviewed Sep 11, 2017

View reviewed changes

CoreFx #22406 Span based APIs - Text Reader Writer

13f9e14

WinCPP force-pushed the span_issue_22406 branch from fa84df2 to 13f9e14 Compare September 20, 2017 14:58

pjanotti merged commit c09ce65 into dotnet:master Sep 21, 2017

karelz added this to the 2.1.0 milestone Oct 11, 2017

WinCPP deleted the span_issue_22406 branch September 9, 2019 03:41

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022

CoreFx dotnet/corefx#22406 Span based APIs - Text Reader Writer (dotn…

cc30fac

…et/corefx#23786) Commit migrated from dotnet/corefx@c09ce65

		protected static object thirdObject = (object)"<third object>";
		protected static object[] multipleObjects = new object[] { firstObject, secondObject, thirdObject };

TextReader/Writer Span overloads and tests #23786

TextReader/Writer Span overloads and tests #23786

Conversation

WinCPP commented Sep 4, 2017 • edited by stephentoub Loading

WinCPP commented Sep 4, 2017

Choose a reason for hiding this comment

WinCPP commented Sep 5, 2017

WinCPP commented Sep 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WinCPP commented Sep 5, 2017

stephentoub commented Sep 5, 2017

WinCPP commented Sep 5, 2017

Choose a reason for hiding this comment

WinCPP Sep 5, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephentoub left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmitche commented Sep 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danmoseley left a comment

Choose a reason for hiding this comment

danmoseley commented Sep 8, 2017

WinCPP commented Sep 8, 2017

Choose a reason for hiding this comment

WinCPP commented Sep 8, 2017

mmitche commented Sep 8, 2017

WinCPP commented Sep 8, 2017

WinCPP commented Sep 8, 2017

WinCPP commented Sep 8, 2017

WinCPP commented Sep 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WinCPP Sep 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephentoub left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WinCPP commented Sep 4, 2017 •

edited by stephentoub

Loading

WinCPP Sep 5, 2017 •

edited

Loading

WinCPP Sep 16, 2017 •

edited

Loading