Re: [Mono-dev] [Patch] NameObjectCollectionBase, HttpCookieCollection

2007-12-20 Thread Juraj Skripsky
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

2007-12-20 Thread Gert Driesen
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

2007-12-21 Thread Juraj Skripsky
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

2007-12-21 Thread Konstantin Triger
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

2007-12-21 Thread Juraj Skripsky
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

2007-12-21 Thread Gert Driesen
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

2007-12-22 Thread Gert Driesen
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

2007-12-29 Thread Juraj Skripsky
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

2008-01-07 Thread Juraj Skripsky
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

2008-01-07 Thread Miguel de Icaza
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

2008-01-07 Thread Juraj Skripsky
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

2008-01-07 Thread Juraj Skripsky
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