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)<m_collection.Count)?true:false;
+ return ((++m_position) < m_collection.Count);
}
public void Reset()
{
@@ -128,17 +128,26 @@
}
// ICollection methods -----------------------------------
- void ICollection.CopyTo(Array arr, int index)
+ void ICollection.CopyTo (Array array, int arrayIndex)
{
- if (arr==null)
- throw new ArgumentNullException("array can't be null");
- IEnumerator en = this.GetEnumerator();
- int i = index;
- while (en.MoveNext())
- {
- arr.SetValue(en.Current,i);
- i++;
- }
+ if (null == array)
+ throw new ArgumentNullException ("array");
+
+ if (arrayIndex < 0)
+ throw new ArgumentOutOfRangeException ("arrayIndex");
+
+ if (array.Rank > 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
Index: System/System.Collections.Specialized/ChangeLog
===================================================================
--- System/System.Collections.Specialized/ChangeLog (revision 91612)
+++ System/System.Collections.Specialized/ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2007-12-19 Juraj Skripsky <[EMAIL PROTECTED]>
+
+ * NameObjectCollectionBase.cs (CopyTo): Add argument checking,
+ replace use of enumerator by for-loop.
+
2007-04-29 Ilya Kharmatsky <[EMAIL PROTECTED]>
* NameValueCollection.cs: Proper exception handling in several
Index: System/Test/System.Collections.Specialized/NameObjectCollectionBaseTest.cs
===================================================================
--- System/Test/System.Collections.Specialized/NameObjectCollectionBaseTest.cs (revision 91612)
+++ System/Test/System.Collections.Specialized/NameObjectCollectionBaseTest.cs (working copy)
@@ -452,5 +452,74 @@
Assert.AreEqual ("string1", array[0], "[0]");
Assert.AreEqual ("string2", array[1], "[1]");
}
+
+ [Test]
+ public void CopyTo_Null ()
+ {
+ UnitTestNameObjectCollectionBase c = new UnitTestNameObjectCollectionBase ();
+ try {
+ ((ICollection)c).CopyTo (null, 0);
+ Assert.Fail ("#1");
+ } catch(ArgumentNullException ex) {
+ Assert.AreEqual (typeof (ArgumentNullException), ex.GetType (), "#2");
+ Assert.IsNull (ex.InnerException, "#3");
+ Assert.IsNotNull (ex.Message, "#4");
+ Assert.IsNotNull (ex.ParamName, "#5");
+ }
+ }
+
+ [Test]
+ public void CopyTo_NegativeIndex ()
+ {
+ string [] array = new string [1];
+ UnitTestNameObjectCollectionBase c = new UnitTestNameObjectCollectionBase ();
+ c.Add ("1", "mono");
+ try {
+ ((ICollection)c).CopyTo (array, -1);
+ Assert.Fail ("#1");
+ } catch(ArgumentOutOfRangeException ex) {
+ Assert.AreEqual (typeof (ArgumentOutOfRangeException), ex.GetType (), "#2");
+ Assert.IsNull (ex.InnerException, "#3");
+ Assert.IsNotNull (ex.Message, "#4");
+ Assert.IsNotNull (ex.ParamName, "#5");
+ }
+ }
+
+ [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");
+ }
+ }
+
+ [Test]
+ public void CopyTo_MultipleDimensionStringArray ()
+ {
+ string [,] matrix = new string [1,1];
+ UnitTestNameObjectCollectionBase c = new UnitTestNameObjectCollectionBase ();
+ c.Add ("1", "mono");
+ try {
+ ((ICollection)c).CopyTo (matrix, 0);
+ Assert.Fail ("#1");
+ } catch (ArgumentException ex) {
+ Assert.AreEqual (typeof (ArgumentException), ex.GetType (), "#2");
+ Assert.IsNull (ex.InnerException, "#3");
+ Assert.IsNotNull (ex.Message, "#4");
+ Assert.IsNotNull (ex.ParamName, "#5");
+ }
+ }
}
}
Index: System/Test/System.Collections.Specialized/ChangeLog
===================================================================
--- System/Test/System.Collections.Specialized/ChangeLog (revision 91612)
+++ System/Test/System.Collections.Specialized/ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2007-12-19 Juraj Skripsky <[EMAIL PROTECTED]>
+
+ * NameObjectCollectionBaseTest.cs: Add tests for argument checking in
+ CopyTo.
+
2007-04-29 Ilya Kharmatsky <[EMAIL PROTECTED]>
* NameValueCollectionTest.cs: Added test which checks proper exception
_______________________________________________
Mono-devel-list mailing list
Mono-devel-list@lists.ximian.com
http://lists.ximian.com/mailman/listinfo/mono-devel-list
