[buildtest] Are there obsolete fields in the test's description? (Was: Re: [bti][p-unit] finally, it is ready)

[Stepan Mishura <st...@gmail.com>]

Hi Alexei,

Thanks for your comments - so your idea that "date-of-creation" and
"authors" fields in the test's description became obsolete.

See my comment below

On 10/29/07, Alexei Fedotov <al...@gmail.com> wrote:
> Hello Stepan,
> I'm gratefull for your interest to this patch.
>
> >  why did you remove logging for "date-of-creation" and
> > "authors" field?
>
> I believe these fields are now excessive. A date of creation may be
> got from svn as well as an author. I was not a supporter or author
> removal idea because svn lists only committers, but this was accepted
> anyway, and now we should to follow.
>

- date of creation != date of first check in
- list of authors != list of committers

So I'd call these fields (as well the corresponding options) -
developer's fields.

> I agree that obsolete warnings may be removed by decreasing logging
> level, but this is not a correct way to follow to my personal point of
> view.
>

What made them "obsolete"?

> > Is is necessary for p-unit integration? I guess no -
> > you can change logging level (or fix tests description files) to avoid
> > unwanted logging.
>
> And you are correct. It is not necessary for p-unit integration. The
> goal is not p-unit integration at all.
>The goal is adding new tests
> and integrating them seamlessly, and absence of obsolete warnings is
> important for seamless integration.

I'm not sure that for adding new tests these warning have to be removed.

Also please keep in mind that the harness is used by several suites
and developers. I think if some options/fields/functionality became
obsolete (and should be removed due to different reasons) then it is
better to discuss it first. If there are no objections then we can
remove it. And it should be removed entirely, not partly as your patch
does.

In this case I don't know if "date-of-creation" and "authors" fields
are useful in test's description or they put additional burden on
developers. So I've changed the subject to to hear opinions of the
harness's users .

Thanks,
Stepan.

>
> Thanks!
On 10/29/07, Stepan Mishura <st...@gmail.com> wrote:
> On 10/26/07, Alexei Fedotov <al...@gmail.com> wrote:
> > Stepan, sure. Thank you for your interest to this patch.
> >
> > For reviewer convinience the harness patch is split into two:
> > Harmony-style formatting changes were separated from removing two
> > warning outputs and authorship [1]. While I respect a contribution of
> > Andrey Tyuryushkin, I have learned the the author removal trick from
> > Sveta.
> >
>
> Alexei,
>
> Thanks for providing new patch that free from formatting changes -
> that made the review easier. Now I have a question to your updates to
> the harness: why did you remove logging for "date-of-creation" and
> "authors" field? Is is necessary for p-unit integration? I guess no -
> you can change logging level (or fix tests description files) to avoid
> unwanted logging.
>
> Thanks,
> Stepan.
>
> > Thanks.
> >
> > [1] http://issues.apache.org/jira/secure/attachment/12368462/harness_review.patch

Re: [buildtest] Are there obsolete fields in the test's description? (Was: Re: [bti][p-unit] finally, it is ready)

[Alexei Fedotov <al...@gmail.com>]

BTW, Stepan,
I have prepared a patch for the stress test suite BTI adapter. The
patch delegates tool compilation from the adapter to the build file
(tools are needed for a stand-alone mode as well), and adds invoking
p-unit checkout target. Could you please look into it?

Thanks.

http://issues.apache.org/jira/browse/HARMONY-5048


On 11/1/07, Alexei Fedotov <al...@gmail.com> wrote:
> Stepan,
> Ok, I will live with these warnings produced by the harness.
>
> Thanks.
>
>
>
>
> On 11/1/07, Stepan Mishura <st...@gmail.com> wrote:
> > Hi Alexei,
> >
> > On 10/30/07, Alexei Fedotov <al...@gmail.com> wrote:
> > > Stepan,
> > >
> > > You asked:
> > > > What made them [fields] "obsolete"?
> > >
> > > Actually the thing I'm suggesting by my patch is to make these fields
> > > "optional".
> >
> > This is not "true" - they are optional. It is possible to omit them in
> > the test's description - the harness runs the test, only warning is
> > printed. For example, if you omit 'testID' then exception is thrown
> > and the test is not run.
> >
> > > I believe I may want to make Harmony becoming more
> > > convenient for a test developer I am.
> > >
> > > The author field was made obsolete by Geir's decision to keep authors
> > > outside of source code (remember removing Ivan Volosyuk from
> > > interpreter sources?) This decision was discussed a bit, but there
> > > were no strong arguments against.
> > >
> >
> > IMO, 'authors' names in code source and 'authors' field in test's
> > description are two different things. I wouldn't mix them. Yes, there
> > is the agreement (OK to remove) about the first one and the second one
> > hasn't been discussed before.
> >
> > > I don't see a reason of keeping date-of-creation attribute as a
> > > mandatory field. The following argument is a bit stronger: this
> > > argument is duplicated with another one and should be completely
> > > removed. The first file modification is usually done at the Day of
> > > Creation, and the current format allows several modification dates.
> > >
> >
> > Again, I believe that if there is a reason to remove some feature then
> > the removal should be entire and complete. We shouldn't remove only
> > warnings and leave other debris of functionality in the harness.
> >
> > For example, currently the harness has the option to select tests for
> > running based on authors name. If I want to run all tests created by
> > you I should pass to the harness something like: "-execopt
> > Selector:exclude:Author Fedotov". Also in this case a warning about
> > tests that were not selected because of missing authors field is very
> > helpful. And your patch removes it. The same for "creation-date".
> >
> > > Thanks.
> > >
> > > BTW, to make this discussion a bit more interesting for techies, let
> > > me add here a discussion of memory cleanup algorithm from our chat
> > > with Andrew:
> > >
> > > Andrew: Does it make sense to invoke System.gc() multiple times to
> > > release memory completely?
> > >
> > > me: I have the following assertion in my tests: allocated == finalized
> > > and there is no other way to check that all objects are finalized than
> > > to invoke gc and to check that amount of available memory no longer
> > > increase (I also check that max chunk size stabilizes)
> > >
> > > Andrew: so to release memory completely, which approach is better? 1.
> > > invoke gc multiple times; 2. gc, thread.sleep
> > >
> > > me: I believe I combine both:
> > >  sleep does actual finalization work :-)
> > >  gc() is for check
> > >
> >
> > I'd like to add a couple of notes here: If you like to discuss stress
> > tests details it is better to start new thread with corresponding
> > topic. And before revealing private conversation details please make
> > sure to get consent from all parties involved.
> >
> > Thanks,
> > Stepan.
> >
> > <SNIP>
> >
>
>
> --
> With best regards,
> Alexei,
> ESSD, Intel
>


-- 
With best regards,
Alexei,
ESSD, Intel

Re: [buildtest] Are there obsolete fields in the test's description? (Was: Re: [bti][p-unit] finally, it is ready)

[Alexei Fedotov <al...@gmail.com>]

Stepan,
Ok, I will live with these warnings produced by the harness.

Thanks.




On 11/1/07, Stepan Mishura <st...@gmail.com> wrote:
> Hi Alexei,
>
> On 10/30/07, Alexei Fedotov <al...@gmail.com> wrote:
> > Stepan,
> >
> > You asked:
> > > What made them [fields] "obsolete"?
> >
> > Actually the thing I'm suggesting by my patch is to make these fields
> > "optional".
>
> This is not "true" - they are optional. It is possible to omit them in
> the test's description - the harness runs the test, only warning is
> printed. For example, if you omit 'testID' then exception is thrown
> and the test is not run.
>
> > I believe I may want to make Harmony becoming more
> > convenient for a test developer I am.
> >
> > The author field was made obsolete by Geir's decision to keep authors
> > outside of source code (remember removing Ivan Volosyuk from
> > interpreter sources?) This decision was discussed a bit, but there
> > were no strong arguments against.
> >
>
> IMO, 'authors' names in code source and 'authors' field in test's
> description are two different things. I wouldn't mix them. Yes, there
> is the agreement (OK to remove) about the first one and the second one
> hasn't been discussed before.
>
> > I don't see a reason of keeping date-of-creation attribute as a
> > mandatory field. The following argument is a bit stronger: this
> > argument is duplicated with another one and should be completely
> > removed. The first file modification is usually done at the Day of
> > Creation, and the current format allows several modification dates.
> >
>
> Again, I believe that if there is a reason to remove some feature then
> the removal should be entire and complete. We shouldn't remove only
> warnings and leave other debris of functionality in the harness.
>
> For example, currently the harness has the option to select tests for
> running based on authors name. If I want to run all tests created by
> you I should pass to the harness something like: "-execopt
> Selector:exclude:Author Fedotov". Also in this case a warning about
> tests that were not selected because of missing authors field is very
> helpful. And your patch removes it. The same for "creation-date".
>
> > Thanks.
> >
> > BTW, to make this discussion a bit more interesting for techies, let
> > me add here a discussion of memory cleanup algorithm from our chat
> > with Andrew:
> >
> > Andrew: Does it make sense to invoke System.gc() multiple times to
> > release memory completely?
> >
> > me: I have the following assertion in my tests: allocated == finalized
> > and there is no other way to check that all objects are finalized than
> > to invoke gc and to check that amount of available memory no longer
> > increase (I also check that max chunk size stabilizes)
> >
> > Andrew: so to release memory completely, which approach is better? 1.
> > invoke gc multiple times; 2. gc, thread.sleep
> >
> > me: I believe I combine both:
> >  sleep does actual finalization work :-)
> >  gc() is for check
> >
>
> I'd like to add a couple of notes here: If you like to discuss stress
> tests details it is better to start new thread with corresponding
> topic. And before revealing private conversation details please make
> sure to get consent from all parties involved.
>
> Thanks,
> Stepan.
>
> <SNIP>
>


-- 
With best regards,
Alexei,
ESSD, Intel

Re: [buildtest] Are there obsolete fields in the test's description? (Was: Re: [bti][p-unit] finally, it is ready)

[Tim Ellison <t....@gmail.com>]

Stepan Mishura wrote:
> For example, currently the harness has the option to select tests for
> running based on authors name. If I want to run all tests created by
> you I should pass to the harness something like: "-execopt
> Selector:exclude:Author Fedotov". Also in this case a warning about
> tests that were not selected because of missing authors field is very
> helpful. And your patch removes it. The same for "creation-date".

I don't think this is a useful feature.  They are just 'harmony tests'
so I see no value in including/excluding tests written by a particular
author.

My 2c.

Regards,
Tim

Re: [buildtest] Are there obsolete fields in the test's description? (Was: Re: [bti][p-unit] finally, it is ready)

[Stepan Mishura <st...@gmail.com>]

Hi Alexei,

On 10/30/07, Alexei Fedotov <al...@gmail.com> wrote:
> Stepan,
>
> You asked:
> > What made them [fields] "obsolete"?
>
> Actually the thing I'm suggesting by my patch is to make these fields
> "optional".

This is not "true" - they are optional. It is possible to omit them in
the test's description - the harness runs the test, only warning is
printed. For example, if you omit 'testID' then exception is thrown
and the test is not run.

> I believe I may want to make Harmony becoming more
> convenient for a test developer I am.
>
> The author field was made obsolete by Geir's decision to keep authors
> outside of source code (remember removing Ivan Volosyuk from
> interpreter sources?) This decision was discussed a bit, but there
> were no strong arguments against.
>

IMO, 'authors' names in code source and 'authors' field in test's
description are two different things. I wouldn't mix them. Yes, there
is the agreement (OK to remove) about the first one and the second one
hasn't been discussed before.

> I don't see a reason of keeping date-of-creation attribute as a
> mandatory field. The following argument is a bit stronger: this
> argument is duplicated with another one and should be completely
> removed. The first file modification is usually done at the Day of
> Creation, and the current format allows several modification dates.
>

Again, I believe that if there is a reason to remove some feature then
the removal should be entire and complete. We shouldn't remove only
warnings and leave other debris of functionality in the harness.

For example, currently the harness has the option to select tests for
running based on authors name. If I want to run all tests created by
you I should pass to the harness something like: "-execopt
Selector:exclude:Author Fedotov". Also in this case a warning about
tests that were not selected because of missing authors field is very
helpful. And your patch removes it. The same for "creation-date".

> Thanks.
>
> BTW, to make this discussion a bit more interesting for techies, let
> me add here a discussion of memory cleanup algorithm from our chat
> with Andrew:
>
> Andrew: Does it make sense to invoke System.gc() multiple times to
> release memory completely?
>
> me: I have the following assertion in my tests: allocated == finalized
> and there is no other way to check that all objects are finalized than
> to invoke gc and to check that amount of available memory no longer
> increase (I also check that max chunk size stabilizes)
>
> Andrew: so to release memory completely, which approach is better? 1.
> invoke gc multiple times; 2. gc, thread.sleep
>
> me: I believe I combine both:
>  sleep does actual finalization work :-)
>  gc() is for check
>

I'd like to add a couple of notes here: If you like to discuss stress
tests details it is better to start new thread with corresponding
topic. And before revealing private conversation details please make
sure to get consent from all parties involved.

Thanks,
Stepan.

<SNIP>

Re: [buildtest] Are there obsolete fields in the test's description? (Was: Re: [bti][p-unit] finally, it is ready)

[Alexei Fedotov <al...@gmail.com>]

Stepan,

You asked:
> What made them [fields] "obsolete"?

Actually the thing I'm suggesting by my patch is to make these fields
"optional". I believe I may want to make Harmony becoming more
convenient for a test developer I am.

The author field was made obsolete by Geir's decision to keep authors
outside of source code (remember removing Ivan Volosyuk from
interpreter sources?) This decision was discussed a bit, but there
were no strong arguments against.

I don't see a reason of keeping date-of-creation attribute as a
mandatory field. The following argument is a bit stronger: this
argument is duplicated with another one and should be completely
removed. The first file modification is usually done at the Day of
Creation, and the current format allows several modification dates.

Thanks.

BTW, to make this discussion a bit more interesting for techies, let
me add here a discussion of memory cleanup algorithm from our chat
with Andrew:

Andrew: Does it make sense to invoke System.gc() multiple times to
release memory completely?

me: I have the following assertion in my tests: allocated == finalized
and there is no other way to check that all objects are finalized than
to invoke gc and to check that amount of available memory no longer
increase (I also check that max chunk size stabilizes)

Andrew: so to release memory completely, which approach is better? 1.
invoke gc multiple times; 2. gc, thread.sleep

me: I believe I combine both:
  sleep does actual finalization work :-)
  gc() is for check

On 10/30/07, Stepan Mishura <st...@gmail.com> wrote:
> Hi Alexei,
>
> Thanks for your comments - so your idea that "date-of-creation" and
> "authors" fields in the test's description became obsolete.
>
> See my comment below
>
> On 10/29/07, Alexei Fedotov <al...@gmail.com> wrote:
> > Hello Stepan,
> > I'm gratefull for your interest to this patch.
> >
> > >  why did you remove logging for "date-of-creation" and
> > > "authors" field?
> >
> > I believe these fields are now excessive. A date of creation may be
> > got from svn as well as an author. I was not a supporter or author
> > removal idea because svn lists only committers, but this was accepted
> > anyway, and now we should to follow.
> >
>
> - date of creation != date of first check in
> - list of authors != list of committers
>
> So I'd call these fields (as well the corresponding options) -
> developer's fields.
>
> > I agree that obsolete warnings may be removed by decreasing logging
> > level, but this is not a correct way to follow to my personal point of
> > view.
> >
>
> What made them "obsolete"?
>
> > > Is is necessary for p-unit integration? I guess no -
> > > you can change logging level (or fix tests description files) to avoid
> > > unwanted logging.
> >
> > And you are correct. It is not necessary for p-unit integration. The
> > goal is not p-unit integration at all.
> >The goal is adding new tests
> > and integrating them seamlessly, and absence of obsolete warnings is
> > important for seamless integration.
>
> I'm not sure that for adding new tests these warning have to be removed.
>
> Also please keep in mind that the harness is used by several suites
> and developers. I think if some options/fields/functionality became
> obsolete (and should be removed due to different reasons) then it is
> better to discuss it first. If there are no objections then we can
> remove it. And it should be removed entirely, not partly as your patch
> does.
>
> In this case I don't know if "date-of-creation" and "authors" fields
> are useful in test's description or they put additional burden on
> developers. So I've changed the subject to to hear opinions of the
> harness's users .
>
> Thanks,
> Stepan.
>
> >
> > Thanks!
> On 10/29/07, Stepan Mishura <st...@gmail.com> wrote:
> > On 10/26/07, Alexei Fedotov <al...@gmail.com> wrote:
> > > Stepan, sure. Thank you for your interest to this patch.
> > >
> > > For reviewer convinience the harness patch is split into two:
> > > Harmony-style formatting changes were separated from removing two
> > > warning outputs and authorship [1]. While I respect a contribution of
> > > Andrey Tyuryushkin, I have learned the the author removal trick from
> > > Sveta.
> > >
> >
> > Alexei,
> >
> > Thanks for providing new patch that free from formatting changes -
> > that made the review easier. Now I have a question to your updates to
> > the harness: why did you remove logging for "date-of-creation" and
> > "authors" field? Is is necessary for p-unit integration? I guess no -
> > you can change logging level (or fix tests description files) to avoid
> > unwanted logging.
> >
> > Thanks,
> > Stepan.
> >
> > > Thanks.
> > >
> > > [1] http://issues.apache.org/jira/secure/attachment/12368462/harness_review.patch
>


-- 
With best regards,
Alexei,
ESSD, Intel