Re: [Mono-dev] [Patch] NameObjectCollectionBase, HttpCookieCollection
Hi Gert, I'm attaching my patches, updated as per your suggestions. - Juraj On Thu, 2007-12-20 at 10:06 +0100, Gert Driesen wrote: > Hi Juraj, > > I'd advise against using ExpectedException when multiple calls are made in > the test, as this may lead to false positives. > > For example: > > [Test] > [ExpectedException (typeof (ArgumentException))] > public void CopyTo_NotEnoughSpace () > { > string [] array = new string [4]; > UnitTestNameObjectCollectionBase c = new > UnitTestNameObjectCollectionBase (); > c.Add ("1", "mono"); > c.Add ("2", "MoNo"); > c.Add ("3", "mOnO"); > c.Add ("4", "MONO"); > ((ICollection)c).CopyTo (array, 2); > } > > If any of the Add methods would lead to an ArgumentException, the test would > pass although you explicitly wanted to check if CopyTo resulted in an > ArgumentException. > > I would advise the following code (which is more bloated, yes): > > [Test] > public void CopyTo_NotEnoughSpace () > { > string [] array = new string [4]; > UnitTestNameObjectCollectionBase c = new > UnitTestNameObjectCollectionBase (); > c.Add ("1", "mono"); > c.Add ("2", "MoNo"); > c.Add ("3", "mOnO"); > c.Add ("4", "MONO"); > try { > ((ICollection)c).CopyTo (array, 2); > Assert.Fail ("#1"); > } catch (ArgumentException ex) { > Assert.AreEqual (typeof (ArgumentException), > ex.GetType (), "#2"); > Assert.IsNull (ex.InnerException, "#3"); > Assert.IsNotNull (ex.Message, "#4"); > Assert.IsNull (ex.ParamName, "#5"); > } > } > > This also allows you to perform additional checks (eg. was there an inner > exception?). > > Gert > > -Original Message- > From: [EMAIL PROTECTED] > [mailto:[EMAIL PROTECTED] On Behalf Of Juraj > Skripsky > Sent: woensdag 19 december 2007 11:27 > To: mono-devel-list@lists.ximian.com > Subject: [Mono-dev] [Patch] NameObjectCollectionBase, HttpCookieCollection > > Hello, > > Attached are three small patches for NameObjectCollectionBase.cs, > NameObjectCollectionBaseTest.cs and HttpCookieCollection.cs. > > All unit tests pass on Mono. Could someone verify that the new unit tests > work on MS.NET? > > May I commit? > > - Juraj > Index: System.Web/System.Web/ChangeLog === --- System.Web/System.Web/ChangeLog (revision 91583) +++ System.Web/System.Web/ChangeLog (working copy) @@ -1,3 +1,7 @@ +2007-12-19 Juraj Skripsky <[EMAIL PROTECTED]> + + * HttpCookieCollection.cs (AllKeys): Use Keys.CopyTo(). + 2007-12-18 Miguel de Icaza <[EMAIL PROTECTED]> * HttpCookieCollection.cs (Get): implement using the indexer to Index: System.Web/System.Web/HttpCookieCollection.cs === --- System.Web/System.Web/HttpCookieCollection.cs (revision 91583) +++ System.Web/System.Web/HttpCookieCollection.cs (working copy) @@ -28,6 +28,7 @@ // WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. // +using System.Collections; using System.Collections.Specialized; using System.Security.Permissions; @@ -161,13 +162,8 @@ public string[] AllKeys { get { -/* XXX another inefficient copy due to - * lack of exposure from the base - * class */ string[] keys = new string [Keys.Count]; -for (int i = 0; i < Keys.Count; i ++) - keys[i] = Keys[i]; - +((ICollection)Keys).CopyTo (keys, 0); return keys; } } Index: System/System.Collections.Specialized/NameObjectCollectionBase.cs === --- System/System.Collections.Specialized/NameObjectCollectionBase.cs (revision 91612) +++ System/System.Collections.Specialized/NameObjectCollectionBase.cs (working copy) @@ -101,7 +101,7 @@ } public bool MoveNext() { -return ((++m_position) 1) + throw new ArgumentException ("array is multidimensional", "array"); + +if ((array.Length > 0) && (arrayIndex >= array.Length)) + throw new ArgumentException ("arrayIndex is equal to or greater than array.Length"); + +ArrayList items = m_collection.m_ItemsArray; +if (arrayIndex + items.Count > array.Length) + throw new ArgumentException ("Not enough room from arrayIndex to end of array for this KeysCollection"); + +for (int i = arrayIndex; i < items.Count; i++) + array.SetValue (((_Item)items [i]).key, i); } bool ICollection.IsSynchronized @@ -360,7 +369,7 @@ void ICollection.CopyTo (Array array, int index) { - (Keys as ICollection).CopyTo (array, index); + ((ICollection)Keys).CopyTo (array, index); } // IDeserializationCallback In
Re: [Mono-dev] [Patch] NameObjectCollectionBase, HttpCookieCollection
Hi Juraj, I made some adjustments to your tests to get them to pass on both .NET 2.0 (SP1) and .NET 1.1. I also improved some existing tests that may require additional changes before they pass on Mono. Gert PS. Sorry if the attachment is binary (Outlook, says it all ...). -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Juraj Skripsky Sent: donderdag 20 december 2007 10:38 To: Gert Driesen Cc: mono-devel-list@lists.ximian.com Subject: Re: [Mono-dev] [Patch] NameObjectCollectionBase,HttpCookieCollection Hi Gert, I'm attaching my patches, updated as per your suggestions. - Juraj On Thu, 2007-12-20 at 10:06 +0100, Gert Driesen wrote: > Hi Juraj, > > I'd advise against using ExpectedException when multiple calls are > made in the test, as this may lead to false positives. > > For example: > > [Test] > [ExpectedException (typeof (ArgumentException))] > public void CopyTo_NotEnoughSpace () > { > string [] array = new string [4]; > UnitTestNameObjectCollectionBase c = new > UnitTestNameObjectCollectionBase (); > c.Add ("1", "mono"); > c.Add ("2", "MoNo"); > c.Add ("3", "mOnO"); > c.Add ("4", "MONO"); > ((ICollection)c).CopyTo (array, 2); > } > > If any of the Add methods would lead to an ArgumentException, the test > would pass although you explicitly wanted to check if CopyTo resulted > in an ArgumentException. > > I would advise the following code (which is more bloated, yes): > > [Test] > public void CopyTo_NotEnoughSpace () > { > string [] array = new string [4]; > UnitTestNameObjectCollectionBase c = new > UnitTestNameObjectCollectionBase (); > c.Add ("1", "mono"); > c.Add ("2", "MoNo"); > c.Add ("3", "mOnO"); > c.Add ("4", "MONO"); > try { > ((ICollection)c).CopyTo (array, 2); > Assert.Fail ("#1"); > } catch (ArgumentException ex) { > Assert.AreEqual (typeof (ArgumentException), ex.GetType (), "#2"); > Assert.IsNull (ex.InnerException, "#3"); > Assert.IsNotNull (ex.Message, "#4"); > Assert.IsNull (ex.ParamName, "#5"); > } > } > > This also allows you to perform additional checks (eg. was there an > inner exception?). > > Gert > > -Original Message- > From: [EMAIL PROTECTED] > [mailto:[EMAIL PROTECTED] On Behalf Of Juraj > Skripsky > Sent: woensdag 19 december 2007 11:27 > To: mono-devel-list@lists.ximian.com > Subject: [Mono-dev] [Patch] NameObjectCollectionBase, > HttpCookieCollection > > Hello, > > Attached are three small patches for NameObjectCollectionBase.cs, > NameObjectCollectionBaseTest.cs and HttpCookieCollection.cs. > > All unit tests pass on Mono. Could someone verify that the new unit > tests work on MS.NET? > > May I commit? > > - Juraj > NameObjectCollectionBaseTest.patch Description: Binary data ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [Patch] NameObjectCollectionBase, HttpCookieCollection
Hi Gert, Thanks for your thorough review! Attached is the lastest and greatest. All unit tests are updated and pass. May I commit? - Juraj On Thu, 2007-12-20 at 18:44 +0100, Gert Driesen wrote: > Hi Juraj, > > I made some adjustments to your tests to get them to pass on both .NET 2.0 > (SP1) and .NET 1.1. > > I also improved some existing tests that may require additional changes > before they pass on Mono. > > Gert > > PS. Sorry if the attachment is binary (Outlook, says it all ...). > > -Original Message- > From: [EMAIL PROTECTED] > [mailto:[EMAIL PROTECTED] On Behalf Of Juraj > Skripsky > Sent: donderdag 20 december 2007 10:38 > To: Gert Driesen > Cc: mono-devel-list@lists.ximian.com > Subject: Re: [Mono-dev] [Patch] > NameObjectCollectionBase,HttpCookieCollection > > Hi Gert, > > I'm attaching my patches, updated as per your suggestions. > > - Juraj > > > On Thu, 2007-12-20 at 10:06 +0100, Gert Driesen wrote: > > Hi Juraj, > > > > I'd advise against using ExpectedException when multiple calls are > > made in the test, as this may lead to false positives. > > > > For example: > > > > [Test] > > [ExpectedException (typeof (ArgumentException))] > > public void CopyTo_NotEnoughSpace () > > { > > string [] array = new string [4]; > > UnitTestNameObjectCollectionBase c = new > > UnitTestNameObjectCollectionBase (); > > c.Add ("1", "mono"); > > c.Add ("2", "MoNo"); > > c.Add ("3", "mOnO"); > > c.Add ("4", "MONO"); > > ((ICollection)c).CopyTo (array, 2); > > } > > > > If any of the Add methods would lead to an ArgumentException, the test > > would pass although you explicitly wanted to check if CopyTo resulted > > in an ArgumentException. > > > > I would advise the following code (which is more bloated, yes): > > > > [Test] > > public void CopyTo_NotEnoughSpace () > > { > > string [] array = new string [4]; > > UnitTestNameObjectCollectionBase c = new > > UnitTestNameObjectCollectionBase (); > > c.Add ("1", "mono"); > > c.Add ("2", "MoNo"); > > c.Add ("3", "mOnO"); > > c.Add ("4", "MONO"); > > try { > > ((ICollection)c).CopyTo (array, 2); > > Assert.Fail ("#1"); > > } catch (ArgumentException ex) { > > Assert.AreEqual (typeof (ArgumentException), > ex.GetType (), "#2"); > > Assert.IsNull (ex.InnerException, "#3"); > > Assert.IsNotNull (ex.Message, "#4"); > > Assert.IsNull (ex.ParamName, "#5"); > > } > > } > > > > This also allows you to perform additional checks (eg. was there an > > inner exception?). > > > > Gert > > > > -Original Message- > > From: [EMAIL PROTECTED] > > [mailto:[EMAIL PROTECTED] On Behalf Of Juraj > > Skripsky > > Sent: woensdag 19 december 2007 11:27 > > To: mono-devel-list@lists.ximian.com > > Subject: [Mono-dev] [Patch] NameObjectCollectionBase, > > HttpCookieCollection > > > > Hello, > > > > Attached are three small patches for NameObjectCollectionBase.cs, > > NameObjectCollectionBaseTest.cs and HttpCookieCollection.cs. > > > > All unit tests pass on Mono. Could someone verify that the new unit > > tests work on MS.NET? > > > > May I commit? > > > > - Juraj > > Index: System.Web/System.Web/ChangeLog === --- System.Web/System.Web/ChangeLog (revision 91583) +++ System.Web/System.Web/ChangeLog (working copy) @@ -1,3 +1,7 @@ +2007-12-19 Juraj Skripsky <[EMAIL PROTECTED]> + + * HttpCookieCollection.cs (AllKeys): Use Keys.CopyTo(). + 2007-12-18 Miguel de Icaza <[EMAIL PROTECTED]> * HttpCookieCollection.cs (Get): implement using the indexer to Index: System.Web/System.Web/HttpCookieCollection.cs === --- System.Web/System.Web/HttpCookieCollection.cs (revision 91583) +++ System.Web/System.Web/HttpCookieCollection.cs (working copy) @@ -28,6 +28,7 @@ // WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SO
Re: [Mono-dev] [Patch] NameObjectCollectionBase, HttpCookieCollection
Title: RE: [Mono-dev] [Patch] NameObjectCollectionBase,HttpCookieCollection Hi Juraj, NameValuCollectionBase.ICollection.CopyTo (Array array, int arrayIndex): Since the key is of type String, the destination array must be either of type string[] or object[], otherwise an InvalidCastException will be thrown. Since string[] inherits from object[], you can 'safely' cast the destination array to object[] and use simple assignment instead of SetValue, which is less efficient. Regards, Konstantin Triger -Original Message- From: [EMAIL PROTECTED] on behalf of Juraj Skripsky Sent: Fri 12/21/2007 13:50 To: Gert Driesen Cc: mono-devel-list@lists.ximian.com Subject: Re: [Mono-dev] [Patch] NameObjectCollectionBase,HttpCookieCollection Hi Gert, Thanks for your thorough review! Attached is the lastest and greatest. All unit tests are updated and pass. May I commit? - Juraj On Thu, 2007-12-20 at 18:44 +0100, Gert Driesen wrote: > Hi Juraj, > > I made some adjustments to your tests to get them to pass on both .NET 2.0 > (SP1) and .NET 1.1. > > I also improved some existing tests that may require additional changes > before they pass on Mono. > > Gert > > PS. Sorry if the attachment is binary (Outlook, says it all ...). > > -Original Message- > From: [EMAIL PROTECTED] > [mailto:[EMAIL PROTECTED]] On Behalf Of Juraj > Skripsky > Sent: donderdag 20 december 2007 10:38 > To: Gert Driesen > Cc: mono-devel-list@lists.ximian.com > Subject: Re: [Mono-dev] [Patch] > NameObjectCollectionBase,HttpCookieCollection > > Hi Gert, > > I'm attaching my patches, updated as per your suggestions. > > - Juraj > > > On Thu, 2007-12-20 at 10:06 +0100, Gert Driesen wrote: > > Hi Juraj, > > > > I'd advise against using ExpectedException when multiple calls are > > made in the test, as this may lead to false positives. > > > > For example: > > > > [Test] > > [ExpectedException (typeof (ArgumentException))] > > public void CopyTo_NotEnoughSpace () > > { > > string [] array = new string [4]; > > UnitTestNameObjectCollectionBase c = new > > UnitTestNameObjectCollectionBase (); > > c.Add ("1", "mono"); > > c.Add ("2", "MoNo"); > > c.Add ("3", "mOnO"); > > c.Add ("4", "MONO"); > > ((ICollection)c).CopyTo (array, 2); > > } > > > > If any of the Add methods would lead to an ArgumentException, the test > > would pass although you explicitly wanted to check if CopyTo resulted > > in an ArgumentException. > > > > I would advise the following code (which is more bloated, yes): > > > > [Test] > > public void CopyTo_NotEnoughSpace () > > { > > string [] array = new string [4]; > > UnitTestNameObjectCollectionBase c = new > > UnitTestNameObjectCollectionBase (); > > c.Add ("1", "mono"); > > c.Add ("2", "MoNo"); > > c.Add ("3", "mOnO"); > > c.Add ("4", "MONO"); > > try { > > ((ICollection)c).CopyTo (array, 2); > > Assert.Fail ("#1"); > > } catch (ArgumentException ex) { > > Assert.AreEqual (typeof (ArgumentException), > ex.GetType (), "#2"); > > Assert.IsNull (ex.InnerException, "#3"); > > Assert.IsNotNull (ex.Message, "#4"); > > Assert.IsNull (ex.ParamName, "#5"); > > } > > } > > > > This also allows you to perform additional checks (eg. was there an > > inner exception?). > > > > Gert > > > > -Original Message- > > From: [EMAIL PROTECTED] > > [mailto:[EMAIL PROTECTED]] On Behalf Of Juraj > > Skripsky > > Sent: woensdag 19 december 2007 11:27 > > To: mono-devel-list@lists.ximian.com > > Subject: [Mono-dev] [Patch] NameObjectCollectionBase, > > HttpCookieCollection > > > > Hello, > > > > Attached are three small patches for NameObjectCollectionBase.cs, > > NameObjectCollectionBaseTest.cs and HttpCookieCollection.cs. > > > > All unit tests pass on Mono. Could someone verify that the new unit > > tests work on MS.NET? > > > > May I commit? > > > > - Juraj > > ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [Patch] NameObjectCollectionBase, HttpCookieCollection
Hi Gert, Sorry, I attached the wrong version. Here you go again, it's the right one this time and also includes the optimization suggested by Konstantin. - Juraj On Fri, 2007-12-21 at 13:09 +0100, Gert Driesen wrote: > Hi Juraj, > > I haven't checked, but are you sure the tests pass on both the 1.0 and the > 2.0 profile. > > I don't see any profile specific code in your patches. > > Gert > > -Original Message- > From: [EMAIL PROTECTED] > [mailto:[EMAIL PROTECTED] On Behalf Of Juraj > Skripsky > Sent: vrijdag 21 december 2007 12:51 > To: Gert Driesen > Cc: mono-devel-list@lists.ximian.com > Subject: Re: [Mono-dev] [Patch] > NameObjectCollectionBase,HttpCookieCollection > > Hi Gert, > > Thanks for your thorough review! Attached is the lastest and greatest. > All unit tests are updated and pass. > > May I commit? > > - Juraj > > > On Thu, 2007-12-20 at 18:44 +0100, Gert Driesen wrote: > > Hi Juraj, > > > > I made some adjustments to your tests to get them to pass on both .NET > > 2.0 > > (SP1) and .NET 1.1. > > > > I also improved some existing tests that may require additional > > changes before they pass on Mono. > > > > Gert > > > > PS. Sorry if the attachment is binary (Outlook, says it all ...). > > > > -Original Message- > > From: [EMAIL PROTECTED] > > [mailto:[EMAIL PROTECTED] On Behalf Of Juraj > > Skripsky > > Sent: donderdag 20 december 2007 10:38 > > To: Gert Driesen > > Cc: mono-devel-list@lists.ximian.com > > Subject: Re: [Mono-dev] [Patch] > > NameObjectCollectionBase,HttpCookieCollection > > > > Hi Gert, > > > > I'm attaching my patches, updated as per your suggestions. > > > > - Juraj > > > > > > On Thu, 2007-12-20 at 10:06 +0100, Gert Driesen wrote: > > > Hi Juraj, > > > > > > I'd advise against using ExpectedException when multiple calls are > > > made in the test, as this may lead to false positives. > > > > > > For example: > > > > > > [Test] > > > [ExpectedException (typeof (ArgumentException))] > > > public void CopyTo_NotEnoughSpace () > > > { > > > string [] array = new string [4]; > > > UnitTestNameObjectCollectionBase c = new > > > UnitTestNameObjectCollectionBase (); > > > c.Add ("1", "mono"); > > > c.Add ("2", "MoNo"); > > > c.Add ("3", "mOnO"); > > > c.Add ("4", "MONO"); > > > ((ICollection)c).CopyTo (array, 2); > > > } > > > > > > If any of the Add methods would lead to an ArgumentException, the > > > test would pass although you explicitly wanted to check if CopyTo > > > resulted in an ArgumentException. > > > > > > I would advise the following code (which is more bloated, yes): > > > > > > [Test] > > > public void CopyTo_NotEnoughSpace () > > > { > > > string [] array = new string [4]; > > > UnitTestNameObjectCollectionBase c = new > > > UnitTestNameObjectCollectionBase (); > > > c.Add ("1", "mono"); > > > c.Add ("2", "MoNo"); > > > c.Add ("3", "mOnO"); > > > c.Add ("4", "MONO"); > > > try { > > > ((ICollection)c).CopyTo (array, 2); > > > Assert.Fail ("#1"); > > > } catch (ArgumentException ex) { > > > Assert.AreEqual (typeof (ArgumentException), > > ex.GetType (), "#2"); > > > Assert.IsNull (ex.InnerException, "#3"); > > > Assert.IsNotNull (ex.Message, "#4"); > > > Assert.IsNull (ex.ParamName, "#5"); > > > } > > > } > > > > > > This also allows you to perform additional checks (eg. was there an > > > inner exception?). > > > > > > Gert > > > > > > -Original Message- > > > From: [EMAIL PROTECTED] > > > [mailto:[EMAIL PROTECTED] On Behalf Of Juraj > > > Skripsky > > > Sent: woensdag 19 december 2007 11:27 > > > To: mono-devel-list@lists.ximian.com > > > Subject: [Mono-dev] [Patch]
Re: [Mono-dev] [Patch] NameObjectCollectionBase, HttpCookieCollection
Hi Juraj, I haven't checked, but are you sure the tests pass on both the 1.0 and the 2.0 profile. I don't see any profile specific code in your patches. Gert -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Juraj Skripsky Sent: vrijdag 21 december 2007 12:51 To: Gert Driesen Cc: mono-devel-list@lists.ximian.com Subject: Re: [Mono-dev] [Patch] NameObjectCollectionBase,HttpCookieCollection Hi Gert, Thanks for your thorough review! Attached is the lastest and greatest. All unit tests are updated and pass. May I commit? - Juraj On Thu, 2007-12-20 at 18:44 +0100, Gert Driesen wrote: > Hi Juraj, > > I made some adjustments to your tests to get them to pass on both .NET > 2.0 > (SP1) and .NET 1.1. > > I also improved some existing tests that may require additional > changes before they pass on Mono. > > Gert > > PS. Sorry if the attachment is binary (Outlook, says it all ...). > > -Original Message- > From: [EMAIL PROTECTED] > [mailto:[EMAIL PROTECTED] On Behalf Of Juraj > Skripsky > Sent: donderdag 20 december 2007 10:38 > To: Gert Driesen > Cc: mono-devel-list@lists.ximian.com > Subject: Re: [Mono-dev] [Patch] > NameObjectCollectionBase,HttpCookieCollection > > Hi Gert, > > I'm attaching my patches, updated as per your suggestions. > > - Juraj > > > On Thu, 2007-12-20 at 10:06 +0100, Gert Driesen wrote: > > Hi Juraj, > > > > I'd advise against using ExpectedException when multiple calls are > > made in the test, as this may lead to false positives. > > > > For example: > > > > [Test] > > [ExpectedException (typeof (ArgumentException))] > > public void CopyTo_NotEnoughSpace () > > { > > string [] array = new string [4]; > > UnitTestNameObjectCollectionBase c = new > > UnitTestNameObjectCollectionBase (); > > c.Add ("1", "mono"); > > c.Add ("2", "MoNo"); > > c.Add ("3", "mOnO"); > > c.Add ("4", "MONO"); > > ((ICollection)c).CopyTo (array, 2); > > } > > > > If any of the Add methods would lead to an ArgumentException, the > > test would pass although you explicitly wanted to check if CopyTo > > resulted in an ArgumentException. > > > > I would advise the following code (which is more bloated, yes): > > > > [Test] > > public void CopyTo_NotEnoughSpace () > > { > > string [] array = new string [4]; > > UnitTestNameObjectCollectionBase c = new > > UnitTestNameObjectCollectionBase (); > > c.Add ("1", "mono"); > > c.Add ("2", "MoNo"); > > c.Add ("3", "mOnO"); > > c.Add ("4", "MONO"); > > try { > > ((ICollection)c).CopyTo (array, 2); > > Assert.Fail ("#1"); > > } catch (ArgumentException ex) { > > Assert.AreEqual (typeof (ArgumentException), > ex.GetType (), "#2"); > > Assert.IsNull (ex.InnerException, "#3"); > > Assert.IsNotNull (ex.Message, "#4"); > > Assert.IsNull (ex.ParamName, "#5"); > > } > > } > > > > This also allows you to perform additional checks (eg. was there an > > inner exception?). > > > > Gert > > > > -Original Message- > > From: [EMAIL PROTECTED] > > [mailto:[EMAIL PROTECTED] On Behalf Of Juraj > > Skripsky > > Sent: woensdag 19 december 2007 11:27 > > To: mono-devel-list@lists.ximian.com > > Subject: [Mono-dev] [Patch] NameObjectCollectionBase, > > HttpCookieCollection > > > > Hello, > > > > Attached are three small patches for NameObjectCollectionBase.cs, > > NameObjectCollectionBaseTest.cs and HttpCookieCollection.cs. > > > > All unit tests pass on Mono. Could someone verify that the new unit > > tests work on MS.NET? > > > > May I commit? > > > > - Juraj > > ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [Patch] NameObjectCollectionBase, HttpCookieCollection
Hi Juraj, It looks ok to me, but since I'm not the maintainer ... Gert -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Juraj Skripsky Sent: vrijdag 21 december 2007 14:34 To: Gert Driesen Cc: Konstantin Triger; mono-devel-list@lists.ximian.com Subject: Re: [Mono-dev] [Patch] NameObjectCollectionBase,HttpCookieCollection Hi Gert, Sorry, I attached the wrong version. Here you go again, it's the right one this time and also includes the optimization suggested by Konstantin. - Juraj On Fri, 2007-12-21 at 13:09 +0100, Gert Driesen wrote: > Hi Juraj, > > I haven't checked, but are you sure the tests pass on both the 1.0 and > the 2.0 profile. > > I don't see any profile specific code in your patches. > > Gert > > -Original Message- > From: [EMAIL PROTECTED] > [mailto:[EMAIL PROTECTED] On Behalf Of Juraj > Skripsky > Sent: vrijdag 21 december 2007 12:51 > To: Gert Driesen > Cc: mono-devel-list@lists.ximian.com > Subject: Re: [Mono-dev] [Patch] > NameObjectCollectionBase,HttpCookieCollection > > Hi Gert, > > Thanks for your thorough review! Attached is the lastest and greatest. > All unit tests are updated and pass. > > May I commit? > > - Juraj > > > On Thu, 2007-12-20 at 18:44 +0100, Gert Driesen wrote: > > Hi Juraj, > > > > I made some adjustments to your tests to get them to pass on both > > .NET 2.0 > > (SP1) and .NET 1.1. > > > > I also improved some existing tests that may require additional > > changes before they pass on Mono. > > > > Gert > > > > PS. Sorry if the attachment is binary (Outlook, says it all ...). > > > > -Original Message- > > From: [EMAIL PROTECTED] > > [mailto:[EMAIL PROTECTED] On Behalf Of Juraj > > Skripsky > > Sent: donderdag 20 december 2007 10:38 > > To: Gert Driesen > > Cc: mono-devel-list@lists.ximian.com > > Subject: Re: [Mono-dev] [Patch] > > NameObjectCollectionBase,HttpCookieCollection > > > > Hi Gert, > > > > I'm attaching my patches, updated as per your suggestions. > > > > - Juraj > > > > > > On Thu, 2007-12-20 at 10:06 +0100, Gert Driesen wrote: > > > Hi Juraj, > > > > > > I'd advise against using ExpectedException when multiple calls are > > > made in the test, as this may lead to false positives. > > > > > > For example: > > > > > > [Test] > > > [ExpectedException (typeof (ArgumentException))] > > > public void CopyTo_NotEnoughSpace () > > > { > > > string [] array = new string [4]; > > > UnitTestNameObjectCollectionBase c = new > > > UnitTestNameObjectCollectionBase (); > > > c.Add ("1", "mono"); > > > c.Add ("2", "MoNo"); > > > c.Add ("3", "mOnO"); > > > c.Add ("4", "MONO"); > > > ((ICollection)c).CopyTo (array, 2); > > > } > > > > > > If any of the Add methods would lead to an ArgumentException, the > > > test would pass although you explicitly wanted to check if CopyTo > > > resulted in an ArgumentException. > > > > > > I would advise the following code (which is more bloated, yes): > > > > > > [Test] > > > public void CopyTo_NotEnoughSpace () > > > { > > > string [] array = new string [4]; > > > UnitTestNameObjectCollectionBase c = new > > > UnitTestNameObjectCollectionBase (); > > > c.Add ("1", "mono"); > > > c.Add ("2", "MoNo"); > > > c.Add ("3", "mOnO"); > > > c.Add ("4", "MONO"); > > > try { > > > ((ICollection)c).CopyTo (array, 2); > > > Assert.Fail ("#1"); > > > } catch (ArgumentException ex) { > > > Assert.AreEqual (typeof (ArgumentException), > > ex.GetType (), "#2"); > > > Assert.IsNull (ex.InnerException, "#3"); > > > Assert.IsNotNull (ex.Message, "#4"); > > > Assert.IsNull (ex.ParamName, "#5"); > > > } > > > } > > > > > > This also allows you to perform additional checks (eg. was there > > > an inner exception?). > > > > > > Gert > > > > > > -Original Message- > > > From: [EMAIL PROTECTED] > > > [mailto:[EMAIL PROTECTED] On Behalf Of > > > Juraj Skripsky > > > Sent: woensdag 19 december 2007 11:27 > > > To: mono-devel-list@lists.ximian.com > > > Subject: [Mono-dev] [Patch] NameObjectCollectionBase, > > > HttpCookieCollection > > > > > > Hello, > > > > > > Attached are three small patches for NameObjectCollectionBase.cs, > > > NameObjectCollectionBaseTest.cs and HttpCookieCollection.cs. > > > > > > All unit tests pass on Mono. Could someone verify that the new > > > unit tests work on MS.NET? > > > > > > May I commit? > > > > > > - Juraj > > > > ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [Patch] NameObjectCollectionBase, HttpCookieCollection
Hi everyone, Who's the maintainer of the classes in System.Collections.Specialized, who can give me the okay to commit my changes (and Gert's unit tests) to NameObjectCollectionBase[Test].cs? (For HttpCookieCollection I know who to bother.) - Juraj On Sat, 2007-12-22 at 10:33 +0100, Gert Driesen wrote: > Hi Juraj, > > It looks ok to me, but since I'm not the maintainer ... > > Gert > > -Original Message- > From: [EMAIL PROTECTED] > [mailto:[EMAIL PROTECTED] On Behalf Of Juraj > Skripsky > Sent: vrijdag 21 december 2007 14:34 > To: Gert Driesen > Cc: Konstantin Triger; mono-devel-list@lists.ximian.com > Subject: Re: [Mono-dev] [Patch] > NameObjectCollectionBase,HttpCookieCollection > > Hi Gert, > > Sorry, I attached the wrong version. > > Here you go again, it's the right one this time and also includes the > optimization suggested by Konstantin. > > - Juraj > > > On Fri, 2007-12-21 at 13:09 +0100, Gert Driesen wrote: > > Hi Juraj, > > > > I haven't checked, but are you sure the tests pass on both the 1.0 and > > the 2.0 profile. > > > > I don't see any profile specific code in your patches. > > > > Gert > > > > -Original Message- > > From: [EMAIL PROTECTED] > > [mailto:[EMAIL PROTECTED] On Behalf Of Juraj > > Skripsky > > Sent: vrijdag 21 december 2007 12:51 > > To: Gert Driesen > > Cc: mono-devel-list@lists.ximian.com > > Subject: Re: [Mono-dev] [Patch] > > NameObjectCollectionBase,HttpCookieCollection > > > > Hi Gert, > > > > Thanks for your thorough review! Attached is the lastest and greatest. > > All unit tests are updated and pass. > > > > May I commit? > > > > - Juraj > > > > > > On Thu, 2007-12-20 at 18:44 +0100, Gert Driesen wrote: > > > Hi Juraj, > > > > > > I made some adjustments to your tests to get them to pass on both > > > .NET 2.0 > > > (SP1) and .NET 1.1. > > > > > > I also improved some existing tests that may require additional > > > changes before they pass on Mono. > > > > > > Gert > > > > > > PS. Sorry if the attachment is binary (Outlook, says it all ...). > > > > > > -Original Message- > > > From: [EMAIL PROTECTED] > > > [mailto:[EMAIL PROTECTED] On Behalf Of Juraj > > > Skripsky > > > Sent: donderdag 20 december 2007 10:38 > > > To: Gert Driesen > > > Cc: mono-devel-list@lists.ximian.com > > > Subject: Re: [Mono-dev] [Patch] > > > NameObjectCollectionBase,HttpCookieCollection > > > > > > Hi Gert, > > > > > > I'm attaching my patches, updated as per your suggestions. > > > > > > - Juraj > > > > > > > > > On Thu, 2007-12-20 at 10:06 +0100, Gert Driesen wrote: > > > > Hi Juraj, > > > > > > > > I'd advise against using ExpectedException when multiple calls are > > > > made in the test, as this may lead to false positives. > > > > > > > > For example: > > > > > > > > [Test] > > > > [ExpectedException (typeof (ArgumentException))] > > > > public void CopyTo_NotEnoughSpace () > > > > { > > > > string [] array = new string [4]; > > > > UnitTestNameObjectCollectionBase c = new > > > > UnitTestNameObjectCollectionBase (); > > > > c.Add ("1", "mono"); > > > > c.Add ("2", "MoNo"); > > > > c.Add ("3", "mOnO"); > > > > c.Add ("4", "MONO"); > > > > ((ICollection)c).CopyTo (array, 2); > > > > } > > > > > > > > If any of the Add methods would lead to an ArgumentException, the > > > > test would pass although you explicitly wanted to check if CopyTo > > > > resulted in an ArgumentException. > > > > > > > > I would advise the following code (which is more bloated, yes): > > > > > > > > [Test] > > > > public void CopyTo_NotEnoughSpace () > > > > { > > > > string [] array = new string [4]; > > > > UnitTestNameObjectCollectionBase c = new > > > > UnitTestNameObjectCollectionBase (
Re: [Mono-dev] [Patch] NameObjectCollectionBase, HttpCookieCollection
Hello again, Now that the holidays are over, my chances should be better: Who's the maintainer of the classes in System.Collections.Specialized, who can give me the okay to commit my changes (and Gert's unit tests) to NameObjectCollectionBase[Test].cs? - Juraj On Sat, 2007-12-29 at 19:50 +0100, Juraj Skripsky wrote: > Hi everyone, > > Who's the maintainer of the classes in System.Collections.Specialized, > who can give me the okay to commit my changes (and Gert's unit tests) to > NameObjectCollectionBase[Test].cs? > > (For HttpCookieCollection I know who to bother.) > > - Juraj > > > On Sat, 2007-12-22 at 10:33 +0100, Gert Driesen wrote: > > Hi Juraj, > > > > It looks ok to me, but since I'm not the maintainer ... > > > > Gert > > ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [Patch] NameObjectCollectionBase, HttpCookieCollection
Hey, > Who's the maintainer of the classes in System.Collections.Specialized, > who can give me the okay to commit my changes (and Gert's unit tests) to > NameObjectCollectionBase[Test].cs? This looks fine, but it lacks ChangeLog entries. Miguel ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [Patch] NameObjectCollectionBase, HttpCookieCollection
Hi Miguel, Look again, they're there (as they were from day one), although they might lead to commit conflicts by now (I'll sort that out of course). My last mail with the patches attached was this: http://lists.ximian.com/pipermail/mono-devel-list/2007-December/026250.html Is that an okay to commit (including Changelog entries) :-)? - Juraj On Mon, 2008-01-07 at 15:12 -0500, Miguel de Icaza wrote: > Hey, > > > Who's the maintainer of the classes in System.Collections.Specialized, > > who can give me the okay to commit my changes (and Gert's unit tests) to > > NameObjectCollectionBase[Test].cs? > > This looks fine, but it lacks ChangeLog entries. > > Miguel > ___ Mono-devel-list mailing list Mono-devel-list@lists.ximian.com http://lists.ximian.com/mailman/listinfo/mono-devel-list
Re: [Mono-dev] [Patch] NameObjectCollectionBase, HttpCookieCollection
Hi Miguel, You'll find the full patch attached. I've added some details to the ChangeLog entries and changed the author of the new units to Gert as he was the one who wrote them (hope he doesn't mind). - Juraj On Mon, 2008-01-07 at 21:23 +0100, Juraj Skripsky wrote: > Hi Miguel, > > Look again, they're there (as they were from day one), although they > might lead to commit conflicts by now (I'll sort that out of course). My > last mail with the patches attached was this: > http://lists.ximian.com/pipermail/mono-devel-list/2007-December/026250.html > > Is that an okay to commit (including Changelog entries) :-)? > > - Juraj > > > On Mon, 2008-01-07 at 15:12 -0500, Miguel de Icaza wrote: > > Hey, > > > > > Who's the maintainer of the classes in System.Collections.Specialized, > > > who can give me the okay to commit my changes (and Gert's unit tests) to > > > NameObjectCollectionBase[Test].cs? > > > > This looks fine, but it lacks ChangeLog entries. > > > > Miguel > > > > ___ > Mono-devel-list mailing list > Mono-devel-list@lists.ximian.com > http://lists.ximian.com/mailman/listinfo/mono-devel-list > Index: System.Collections.Specialized/NameObjectCollectionBase.cs === --- System.Collections.Specialized/NameObjectCollectionBase.cs (revision 92409) +++ System.Collections.Specialized/NameObjectCollectionBase.cs (working copy) @@ -101,7 +101,7 @@ } public bool MoveNext() { -return ((++m_position) 0) && (arrayIndex >= array.Length)) + throw new ArgumentException ("arrayIndex is equal to or greater than array.Length"); + +if (arrayIndex + items.Count > array.Length) + throw new ArgumentException ("Not enough room from arrayIndex to end of array for this KeysCollection"); +#endif + +if (array != null && array.Rank > 1) + throw new ArgumentException ("array is multidimensional"); + +object[] objArray = (object[])array; +for (int i = 0; i < items.Count; i++, arrayIndex++) + objArray [arrayIndex] = ((_Item)items [i]).key; } bool ICollection.IsSynchronized @@ -360,7 +372,7 @@ void ICollection.CopyTo (Array array, int index) { - (Keys as ICollection).CopyTo (array, index); + ((ICollection)Keys).CopyTo (array, index); } // IDeserializationCallback @@ -595,8 +607,10 @@ /// protected void BaseSet( int index, object value ) { +#if NET_2_0 if (this.IsReadOnly) throw new NotSupportedException("Collection is read-only"); +#endif _Item item = (_Item)m_ItemsArray[index]; item.value = value; } @@ -608,8 +622,10 @@ /// The Object that represents the new value of the entry to set. The value can be a null reference protected void BaseSet( string name, object value ) { +#if NET_2_0 if (this.IsReadOnly) throw new NotSupportedException("Collection is read-only"); +#endif _Item item = FindFirstMatchedItem(name); if (item!=null) item.value=value; Index: System.Collections.Specialized/ChangeLog === --- System.Collections.Specialized/ChangeLog (revision 92409) +++ System.Collections.Specialized/ChangeLog (working copy) @@ -1,3 +1,9 @@ +2008-01-07 Juraj Skripsky <[EMAIL PROTECTED]> + + * NameObjectCollectionBase.cs: Add argument checking in CopyTo, + replace use of enumerator by for-loop. Check IsReadOnly in BaseSet() + only on the 2.0 profile. + 2007-04-29 Ilya Kharmatsky <[EMAIL PROTECTED]> * NameValueCollection.cs: Proper exception handling in several Index: Test/System.Collections.Specialized/NameObjectCollectionBaseTest.cs === --- Test/System.Collections.Specialized/NameObjectCollectionBaseTest.cs (revision 92409) +++ Test/System.Collections.Specialized/NameObjectCollectionBaseTest.cs (working copy) @@ -151,8 +151,8 @@ #endif [TestFixture] - public class NameObjectCollectionBaseTest { - + public class NameObjectCollectionBaseTest + { private void CheckICollection (UnitTestNameObjectCollectionBase coll, int count) { ICollection collection = (coll as ICollection); @@ -311,15 +311,25 @@ [ExpectedException (typeof (ArgumentOutOfRangeException))] public void Constructor_Int_Negative () { - new UnitTestNameObjectCollectionBase (-1, CaseInsensitiveHashCodeProvider.DefaultInvariant, CaseInsensitiveComparer.DefaultInvariant); + new UnitTestNameObjectCollectionBase (-1, +CaseInsensitiveHashCodeProvider.DefaultInvariant, +CaseInsensitiveComparer.DefaultInvariant); } [Test] - [ExpectedException (typeof (ArgumentNullException))] - public void GetObjectData_Null () + public void GetObjectData_Info_Null () { UnitTestNameObjectCollectionBase coll = new UnitTestNameObjectCollectionBase (); - coll.GetObjectData (null, new StreamingContext ()); + try { +coll.GetObjectData (null, new StreamingContex