[Mono-dev] [PATCH] TypeDescriptor

[Ivan N. Zlatev]

On Wed, Apr 30, 2008 at 7:36 PM, James Fitzsimons
<james.fitzsimons at gmail.com> wrote:
>
>
>
>
> 2008/4/28 Ivan N. Zlatev <contact at i-nz.net>:
> >
> >
> >
> > On Mon, Apr 28, 2008 at 11:28 PM, James Fitzsimons
> > <james.fitzsimons at gmail.com> wrote:
> > >
> > > 2008/4/28 Ivan N. Zlatev <contact at i-nz.net>:
> > >
> > >
> > > > 2008/4/28 James Fitzsimons <james.fitzsimons at gmail.com>:
> > > >
> > > >
> > > >
> > > > > Hi all,
> > > > >
> > > > > While debugging a problem with Spring.NET over the weekend I
> uncovered
> > > an
> > > > > inconsistency in behaviour between the Mono and Microsoft
> > > implementations of
> > > > > the  GetProperties method of the TypeDescriptor class. Basically the
> > > > > Microsoft only returns properties that have a getter, however Mono
> > > returns
> > > > > write only properties as well.
> > > > >
> > > > > The attached patch contains a fix and a few more unit tests to check
> > > that it
> > > > > works and it doesn't break existing behaviour.
> > > > >
> > > >
> > > > Please fix the following things and resend:
> > > >
> > > > 1) Reformat your patch to match our coding guidelines -
> > > > http://www.mono-project.com/Coding_Guidelines
> > > > 2) Add ChangeLog entries.
> > > > 3) Fix the mixed indentation in the code in the tests you've added.
> > > > 4) Fix the indentation in this hunk:
> > > >
> > > > @@ -427,6 +447,9 @@
> > > >                MyComponent sitedcom = new MyComponent (new MySite ());
> > > >                MyComponent nfscom = new MyComponent (new NoFilterSite
> (new
> > > MyContainer ()));
> > > >                AnotherComponent anothercom = new AnotherComponent ();
> > > > +               TestObject testObject = new TestObject();
> > > > +        PropertyDescriptorCollection properties =
> > > > TypeDescriptor.GetProperties(testObject);
> > > > +
> > > >
> > > >                [Test]
> > > >                public void TestICustomTypeDescriptor ()
> > > >
> > > > Cheers,
> > > > Ivan
> > > >
> > >
> > >
> > > Maybe this time?
> > >
> >
> > Two more things:
> >
> > 1) Refactor all your asserts into 1 test method, say
> > TestGetProperties_WriteOnly  and drop the fields - use local variables
> > inside. No need for PropertyDescriptorExists, just check the
> > propertydescriptorcollection["property"] != null inside that test
> > method like the other tests do. Number the asserts.
> >
> > 2) There is no need for a special TestObject - just add a single
> > write-only property MyComponent and use that. You will also have to
> > add it to the GetProperties_Order test
> >
> > Resend when those are fixed, thanks.
>
> Hi Ivan,
>
> I've done as you requested and checked all tests still pass.
>

I have applied your patch with a further slight modification of the
tests in r102535, thanks.

Ivan.