You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Stepan Mishura <st...@gmail.com> on 2006/03/28 13:46:49 UTC

Regression tests (was: svn commit: r389475 - in /incubator/harmony/enhanced/classlib/trunk/modules/text/src: main/java/java/text/SimpleDateFormat.java test/java/tests/api/java/text/SimpleDateFormatTest.java)

Hi,

We agreed some time ago to mark regression tests with comments that includes
a reference to JIRA issue.
 There were no objections and I believe this is the good rule so I'd like to
ask everybody to follow it.
I've update testing wiki page to fix this agreement (see
http://wiki.apache.org/harmony/Testing_Convention)

Thanks,
Stepan.

>-----Original Message-----
>From: tellison@apache.org [mailto:tellison@apache.org]
>Sent: Tuesday, March 28, 2006 5:57 PM
>To: harmony-commits@incubator.apache.org
>Subject: svn commit: r389475 - in
>/incubator/harmony/enhanced/classlib/trunk/modules/text/src:
>main/java/java/text/SimpleDateFormat.java
>test/java/tests/api/java/text/SimpleDateFormatTest.java
>
>Author: tellison
>Date: Tue Mar 28 02:56:21 2006
>New Revision: 389475
>
>URL: http://svn.apache.org/viewcvs?rev=389475&view=rev
>Log:
>Fix for HARMONY-209 (java.text.SimpleDateFormat.equals() behavior is not
>compatible with RI)
>
>Modified:
>
>incubator/harmony/enhanced/classlib/trunk/modules/text/src/main/java/java/t
>ext/SimpleDateFormat.java
>
>incubator/harmony/enhanced/classlib/trunk/modules/text/src/test/java/tests/
>api/java/text/SimpleDateFormatTest.java
>
>Modified:
>incubator/harmony/enhanced/classlib/trunk/modules/text/src/main/java/java/t
>ext/SimpleDateFormat.java
>URL:
>http://svn.apache.org/viewcvs/incubator/harmony/enhanced/classlib/trunk/mod
>ules/text/src/main/java/java/text/SimpleDateFormat.java?rev=389475&r1=38947
>4&r2=389475&view=diff
>===========================================================================
>===
>---
>incubator/harmony/enhanced/classlib/trunk/modules/text/src/main/java/java/t
>ext/SimpleDateFormat.java (original)
>+++
>incubator/harmony/enhanced/classlib/trunk/modules/text/src/main/java/java/t
>ext/SimpleDateFormat.java Tue Mar 28 02:56:21 2006
>@@ -306,8 +306,10 @@
>  }
>
>  private void appendNumber(StringBuffer buffer, int count, int value)
>{
>+  int minimumIntegerDigits =
>numberFormat.getMinimumIntegerDigits();
>   numberFormat.setMinimumIntegerDigits(count);
>   numberFormat.format(new Integer(value), buffer, new
>FieldPosition(0));
>+  numberFormat.setMinimumIntegerDigits(minimumIntegerDigits);
>  }
>
>  /**
>
>Modified:
>incubator/harmony/enhanced/classlib/trunk/modules/text/src/test/java/tests/
>api/java/text/SimpleDateFormatTest.java
>URL:
>http://svn.apache.org/viewcvs/incubator/harmony/enhanced/classlib/trunk/mod
>ules/text/src/test/java/tests/api/java/text/SimpleDateFormatTest.java?rev=3
>89475&r1=389474&r2=389475&view=diff
>===========================================================================
>===
>---
>incubator/harmony/enhanced/classlib/trunk/modules/text/src/test/java/tests/
>api/java/text/SimpleDateFormatTest.java (original)
>+++
>incubator/harmony/enhanced/classlib/trunk/modules/text/src/test/java/tests/
>api/java/text/SimpleDateFormatTest.java Tue Mar 28 02:56:21 2006
>@@ -301,6 +301,12 @@
>   format.format(new Date());
>   assertTrue("not equal after format", format.equals(clone));
>  }
>+
>+    public void test_equals_afterFormat() {
>+        SimpleDateFormat df = new SimpleDateFormat();
>+        df.format(new Date());
>+        assertEquals(df, new SimpleDateFormat());
>+    }
>
>  /**
>   * @tests
>java.text.SimpleDateFormat#formatToCharacterIterator(java.lang.Object)

--
Thanks,
Stepan Mishura
Intel Middleware Products Division

Re: Regression tests (was: svn commit: r389475 - in /incubator/harmony/enhanced/classlib/trunk/modules/text/src: main/java/java/text/SimpleDateFormat.java test/java/tests/api/java/text/SimpleDateFormatTest.java)

Posted by Geir Magnusson Jr <ge...@pobox.com>.

Mark Hindess wrote:
> Are you expecting committers to do this or submitters?
> 
> Do we really want to raise the bar for submitters?

Yes, because a submitter is on his or her way to be committer.  One of 
the things I look for in potential committers is alignment with the 
other committers.

> 
> As a submitter, my current process is:
> 
> 0) find problem
> 1) create test
> 2) check test fails
> 3) fix code
> 4) check test passes
> 5) create patch
> 6) create jira
> 7) attach patch
> 
> Often I save steps 6 and 7 to submit a bunch of JIRA's in one go.
> It's going to be a pain to make this process:
> 
> 0) find problem
> 1) create test
> 2) check test fails
> 3) fix code
> 4) check test passes
> 5) create JIRA
> 6) edit test
> 7) check test still passes
> 8) create patch
> 9) attach patch
> 
> 
> With the new process I'm much more likely to get interrupted between
> the create and attach step. I think it is better to keep the
> "create/attach" JIRA steps together since then we avoid the
> possibility that someone sees the JIRA and thinks there is no patch
> forthcoming.  (Really I wish that JIRA create/attach could be an
> atomic operation when I have a patch ready.)
> 
> Do we really want to raise the bar for submitters?

Yep. :)

As Tim said, we won't reject because of this, but it sure would be nice.

geir

> 
> Regards,
>  Mark.
> 
> On 3/28/06, Stepan Mishura <st...@gmail.com> wrote:
>> Hi,
>>
>> We agreed some time ago to mark regression tests with comments that includes
>> a reference to JIRA issue.
>>  There were no objections and I believe this is the good rule so I'd like to
>> ask everybody to follow it.
>> I've update testing wiki page to fix this agreement (see
>> http://wiki.apache.org/harmony/Testing_Convention)
>>
>> Thanks,
>> Stepan.
> 
> --
> Mark Hindess <ma...@googlemail.com>
> IBM Java Technology Centre, UK.
> 
> 


Re: Regression tests (was: svn commit: r389475 - in /incubator/harmony/enhanced/classlib/trunk/modules/text/src: main/java/java/text/SimpleDateFormat.java test/java/tests/api/java/text/SimpleDateFormatTest.java)

Posted by Tim Ellison <t....@gmail.com>.
I've never turned down a contribution because it was missing a JIRA
reference.  If you prefer not to include it then that's fine -- that
part is not the gem of information I'm looking for in any contribution.

Regards,
Tim

Mark Hindess wrote:
> Are you expecting committers to do this or submitters?
> 
> Do we really want to raise the bar for submitters?
> 
> As a submitter, my current process is:
> 
> 0) find problem
> 1) create test
> 2) check test fails
> 3) fix code
> 4) check test passes
> 5) create patch
> 6) create jira
> 7) attach patch
> 
> Often I save steps 6 and 7 to submit a bunch of JIRA's in one go.
> It's going to be a pain to make this process:
> 
> 0) find problem
> 1) create test
> 2) check test fails
> 3) fix code
> 4) check test passes
> 5) create JIRA
> 6) edit test
> 7) check test still passes
> 8) create patch
> 9) attach patch
> 
> 
> With the new process I'm much more likely to get interrupted between
> the create and attach step. I think it is better to keep the
> "create/attach" JIRA steps together since then we avoid the
> possibility that someone sees the JIRA and thinks there is no patch
> forthcoming.  (Really I wish that JIRA create/attach could be an
> atomic operation when I have a patch ready.)
> 
> Do we really want to raise the bar for submitters?
> 
> Regards,
>  Mark.
> 
> On 3/28/06, Stepan Mishura <st...@gmail.com> wrote:
>> Hi,
>>
>> We agreed some time ago to mark regression tests with comments that includes
>> a reference to JIRA issue.
>>  There were no objections and I believe this is the good rule so I'd like to
>> ask everybody to follow it.
>> I've update testing wiki page to fix this agreement (see
>> http://wiki.apache.org/harmony/Testing_Convention)
>>
>> Thanks,
>> Stepan.
> 
> --
> Mark Hindess <ma...@googlemail.com>
> IBM Java Technology Centre, UK.
> 

-- 

Tim Ellison (t.p.ellison@gmail.com)
IBM Java technology centre, UK.

Re: Regression tests (was: svn commit: r389475 - in /incubator/harmony/enhanced/classlib/trunk/modules/text/src: main/java/java/text/SimpleDateFormat.java test/java/tests/api/java/text/SimpleDateFormatTest.java)

Posted by Mark Hindess <ma...@googlemail.com>.
Are you expecting committers to do this or submitters?

Do we really want to raise the bar for submitters?

As a submitter, my current process is:

0) find problem
1) create test
2) check test fails
3) fix code
4) check test passes
5) create patch
6) create jira
7) attach patch

Often I save steps 6 and 7 to submit a bunch of JIRA's in one go.
It's going to be a pain to make this process:

0) find problem
1) create test
2) check test fails
3) fix code
4) check test passes
5) create JIRA
6) edit test
7) check test still passes
8) create patch
9) attach patch


With the new process I'm much more likely to get interrupted between
the create and attach step. I think it is better to keep the
"create/attach" JIRA steps together since then we avoid the
possibility that someone sees the JIRA and thinks there is no patch
forthcoming.  (Really I wish that JIRA create/attach could be an
atomic operation when I have a patch ready.)

Do we really want to raise the bar for submitters?

Regards,
 Mark.

On 3/28/06, Stepan Mishura <st...@gmail.com> wrote:
> Hi,
>
> We agreed some time ago to mark regression tests with comments that includes
> a reference to JIRA issue.
>  There were no objections and I believe this is the good rule so I'd like to
> ask everybody to follow it.
> I've update testing wiki page to fix this agreement (see
> http://wiki.apache.org/harmony/Testing_Convention)
>
> Thanks,
> Stepan.

--
Mark Hindess <ma...@googlemail.com>
IBM Java Technology Centre, UK.

Re: Regression tests (was: svn commit: r389475 - in /incubator/harmony/enhanced/classlib/trunk/modules/text/src: main/java/java/text/SimpleDateFormat.java test/java/tests/api/java/text/SimpleDateFormatTest.java)

Posted by Alex Orlov <am...@gmail.com>.
On 3/28/06, Mark Hindess <ma...@googlemail.com> wrote:
> I'm not convinced.
>
> Having the JIRA reference for non-contentious tests - such as an API
> test that passes on the reference implementation - seems a little
> redundant since no one will ever bother to look it up.  If a test was
> contentious then I'd rather see a brief summary/explanation with the
> code (perhaps referencing the JIRA) than to have only the JIRA
> reference.  Having just the JIRA reference seems like a poor shortcut for
> the important cases and needless clutter for the rest.
>
> If we really need the information later it is in the svn log anyway?
> For instance, I just did:
>
> bash$ svn blame \
> modules/text/src/test/java/tests/api/java/text/SimpleDateFormatTest.java| \
> grep test_equals_afterFormat
> 389475   tellison     public void test_equals_afterFormat() {
> bash$ svn log -r 389475
> ------------------------------------------------------------------------
> r389475 | tellison | 2006-03-28 11:56:21 +0100 (Tue, 28 Mar 2006) | 2 lines
>
> Fix for HARMONY-209 (java.text.SimpleDateFormat.equals() behavior is
> not compatible with RI)
>
> ------------------------------------------------------------------------
>
> Granted it gets a little more difficult over time, but the svn log
> doesn't go missing or get deleted accidentally, forgottent, etc.  I
> prefer to let
> the version control system store this kind of metadata (since that is
> what it is intended for) rather than cluttering the code with it.

When the tests are run and some of them will fail it will be quite
convenient to look in the test sources and see its Java code and
whether it contains reference to the corresponding JIRA issue.
Otherwise for each unit test failure someone will have to look in CVS
log even if this is not regression test indeed.

However I agree it makes perfect sense to add brief
summary/explanation in addition to JIRA code.

--
Alex Orlov,
Intel Middleware Products Division.

>
>
> Regards,
> Mark.
>
> On 3/28/06, Tim Ellison <t....@gmail.com> wrote:
> > Yep, I agree this is a good convention. I missed it here but will go and
> > fix it.
> >
> > Thanks
> > Tim
> >
> > Stepan Mishura wrote:
> > > Hi,
> > >
> > > We agreed some time ago to mark regression tests with comments that includes
> > > a reference to JIRA issue.
> > >  There were no objections and I believe this is the good rule so I'd like to
> > > ask everybody to follow it.
> > > I've update testing wiki page to fix this agreement (see
> > > http://wiki.apache.org/harmony/Testing_Convention)
> > >
> > > Thanks,
> > > Stepan.
>
> --
> Mark Hindess <ma...@googlemail.com>
> IBM Java Technology Centre, UK.
>

Re: Regression tests (was: svn commit: r389475 - in /incubator/harmony/enhanced/classlib/trunk/modules/text/src: main/java/java/text/SimpleDateFormat.java test/java/tests/api/java/text/SimpleDateFormatTest.java)

Posted by Mark Hindess <ma...@googlemail.com>.
I'm not convinced.

Having the JIRA reference for non-contentious tests - such as an API
test that passes on the reference implementation - seems a little
redundant since no one will ever bother to look it up.  If a test was
contentious then I'd rather see a brief summary/explanation with the
code (perhaps referencing the JIRA) than to have only the JIRA
reference.  Having just the JIRA reference seems like a poor shortcut for
the important cases and needless clutter for the rest.

If we really need the information later it is in the svn log anyway? 
For instance, I just did:

bash$ svn blame \
  modules/text/src/test/java/tests/api/java/text/SimpleDateFormatTest.java| \
  grep test_equals_afterFormat
389475   tellison     public void test_equals_afterFormat() {
bash$ svn log -r 389475
------------------------------------------------------------------------
r389475 | tellison | 2006-03-28 11:56:21 +0100 (Tue, 28 Mar 2006) | 2 lines

Fix for HARMONY-209 (java.text.SimpleDateFormat.equals() behavior is
not compatible with RI)

------------------------------------------------------------------------

Granted it gets a little more difficult over time, but the svn log
doesn't go missing or get deleted accidentally, forgottent, etc.  I
prefer to let
the version control system store this kind of metadata (since that is
what it is intended for) rather than cluttering the code with it.


Regards,
 Mark.

On 3/28/06, Tim Ellison <t....@gmail.com> wrote:
> Yep, I agree this is a good convention. I missed it here but will go and
> fix it.
>
> Thanks
> Tim
>
> Stepan Mishura wrote:
> > Hi,
> >
> > We agreed some time ago to mark regression tests with comments that includes
> > a reference to JIRA issue.
> >  There were no objections and I believe this is the good rule so I'd like to
> > ask everybody to follow it.
> > I've update testing wiki page to fix this agreement (see
> > http://wiki.apache.org/harmony/Testing_Convention)
> >
> > Thanks,
> > Stepan.

--
Mark Hindess <ma...@googlemail.com>
IBM Java Technology Centre, UK.

Re: Regression tests (was: svn commit: r389475 - in /incubator/harmony/enhanced/classlib/trunk/modules/text/src: main/java/java/text/SimpleDateFormat.java test/java/tests/api/java/text/SimpleDateFormatTest.java)

Posted by Tim Ellison <t....@gmail.com>.
Yep, I agree this is a good convention. I missed it here but will go and
fix it.

Thanks
Tim

Stepan Mishura wrote:
> Hi,
> 
> We agreed some time ago to mark regression tests with comments that includes
> a reference to JIRA issue.
>  There were no objections and I believe this is the good rule so I'd like to
> ask everybody to follow it.
> I've update testing wiki page to fix this agreement (see
> http://wiki.apache.org/harmony/Testing_Convention)
> 
> Thanks,
> Stepan.
> 
>> -----Original Message-----
>> From: tellison@apache.org [mailto:tellison@apache.org]
>> Sent: Tuesday, March 28, 2006 5:57 PM
>> To: harmony-commits@incubator.apache.org
>> Subject: svn commit: r389475 - in
>> /incubator/harmony/enhanced/classlib/trunk/modules/text/src:
>> main/java/java/text/SimpleDateFormat.java
>> test/java/tests/api/java/text/SimpleDateFormatTest.java
>>
>> Author: tellison
>> Date: Tue Mar 28 02:56:21 2006
>> New Revision: 389475
>>
>> URL: http://svn.apache.org/viewcvs?rev=389475&view=rev
>> Log:
>> Fix for HARMONY-209 (java.text.SimpleDateFormat.equals() behavior is not
>> compatible with RI)
>>
>> Modified:
>>
>> incubator/harmony/enhanced/classlib/trunk/modules/text/src/main/java/java/t
>> ext/SimpleDateFormat.java
>>
>> incubator/harmony/enhanced/classlib/trunk/modules/text/src/test/java/tests/
>> api/java/text/SimpleDateFormatTest.java
>>
>> Modified:
>> incubator/harmony/enhanced/classlib/trunk/modules/text/src/main/java/java/t
>> ext/SimpleDateFormat.java
>> URL:
>> http://svn.apache.org/viewcvs/incubator/harmony/enhanced/classlib/trunk/mod
>> ules/text/src/main/java/java/text/SimpleDateFormat.java?rev=389475&r1=38947
>> 4&r2=389475&view=diff
>> ===========================================================================
>> ===
>> ---
>> incubator/harmony/enhanced/classlib/trunk/modules/text/src/main/java/java/t
>> ext/SimpleDateFormat.java (original)
>> +++
>> incubator/harmony/enhanced/classlib/trunk/modules/text/src/main/java/java/t
>> ext/SimpleDateFormat.java Tue Mar 28 02:56:21 2006
>> @@ -306,8 +306,10 @@
>>  }
>>
>>  private void appendNumber(StringBuffer buffer, int count, int value)
>> {
>> +  int minimumIntegerDigits =
>> numberFormat.getMinimumIntegerDigits();
>>   numberFormat.setMinimumIntegerDigits(count);
>>   numberFormat.format(new Integer(value), buffer, new
>> FieldPosition(0));
>> +  numberFormat.setMinimumIntegerDigits(minimumIntegerDigits);
>>  }
>>
>>  /**
>>
>> Modified:
>> incubator/harmony/enhanced/classlib/trunk/modules/text/src/test/java/tests/
>> api/java/text/SimpleDateFormatTest.java
>> URL:
>> http://svn.apache.org/viewcvs/incubator/harmony/enhanced/classlib/trunk/mod
>> ules/text/src/test/java/tests/api/java/text/SimpleDateFormatTest.java?rev=3
>> 89475&r1=389474&r2=389475&view=diff
>> ===========================================================================
>> ===
>> ---
>> incubator/harmony/enhanced/classlib/trunk/modules/text/src/test/java/tests/
>> api/java/text/SimpleDateFormatTest.java (original)
>> +++
>> incubator/harmony/enhanced/classlib/trunk/modules/text/src/test/java/tests/
>> api/java/text/SimpleDateFormatTest.java Tue Mar 28 02:56:21 2006
>> @@ -301,6 +301,12 @@
>>   format.format(new Date());
>>   assertTrue("not equal after format", format.equals(clone));
>>  }
>> +
>> +    public void test_equals_afterFormat() {
>> +        SimpleDateFormat df = new SimpleDateFormat();
>> +        df.format(new Date());
>> +        assertEquals(df, new SimpleDateFormat());
>> +    }
>>
>>  /**
>>   * @tests
>> java.text.SimpleDateFormat#formatToCharacterIterator(java.lang.Object)
> 
> --
> Thanks,
> Stepan Mishura
> Intel Middleware Products Division
> 

-- 

Tim Ellison (t.p.ellison@gmail.com)
IBM Java technology centre, UK.