Re: [boost] Re: Regression testing goes horribly wrong! (+ patch)

From: Aleksey Gurtovoy (agurtovoy_at_[hidden])
Date: 2004-07-10 21:46:38

Guillaume Melquiond writes:
> Le sam 10/07/2004 à 10:01, Aleksey Gurtovoy a écrit :
>> Guillaume Melquiond writes:
>> > Hi,
>> 
>> Hi Guillaume,
>> 
>> >
>> > If you take a look at the regression logs [1], you will see a few
>> > problems.
>> >
>> > First, a lot of test types [2] are missing. The "bind" or
>> > "date-time" libraries don't have any value in the test type
>> > column. Other libraries like the "array" library do have values in
>> > this column, however.
>> 
>> This was fixed in the CVS about two weeks ago (see
>> http://tinyurl.com/36v6h#dont_redirect, for instance); simple
>> rebuilding "process_jam_log" executable will take care of it (I would
>> have posted a notice about this fix if I didn't forget that unlike us
>> not everybody rebuilds "process_jam_log" on every run).
>
> That's strange. I do use the CVS version and I did rebuild the
> process_jam_log. 

Did you do a clean run?

> Moreover, anybody that uses the run_tests.sh script to
> run the regression testing should have an up-to-date version of
> process_jam_log ("step 2: rebuild the regression test helper programs if
> required"). I think your fix was sufficient was for you but not for
> everybody (don't ask me why :).

Given that we remove all binaries and sources and rebuild everything
from scratch on every run, I'm inclined to think that our fix works
and that the problems you see are caused by a stale environment.

If you still can reproduce the problem on a clean state, you can
always step through the process_jam_log code in debugger and see how these
empty test types appear there.

>
>> > Second, the name of some "nested" libraries are wrong. For example,
>> > the "algorithm/string" is shown as "algorithm", the
>> > "numeric/conversion" and "numeric/interval" libraries are both mixed
>> > under "numeric". But the "numeric/ublas" is correctly named.
>> 
>> This is still an open issue.
>
> It's because process_jam_log only try to look if a "sublibs" directory
> exists when it does not have a library name. It does not have a library
> name when the test lies in "bin/boost/status", it is the case for
> "ublas" for example. However, for "conversion", the test lies in
> "bin/boost/libs/numeric/conversion/test". So the program has a library
> name which is "numeric" and it does not bother testing if "sublibs"
> exists. It's what my patch does, I force process_jam_log to always scan
> for a "sublibs" directory:
>
>
> -      if ( library_name.empty() )
> +      if ( !info.file_path.empty() )
>          library_name = test_path_to_library_name( info.file_path );
>
>
> As you can see, process_jam_log was only testing for "sublibs" if
> library_name is empty. That was obviously wrong.

OK, indeed, this is a right change. In fact, it fixes the issue entirely.
Tested and committed, will be reflected in the next run.

[..]

>> Unfortunately, the patch is no good. In particular, it breaks the case
>> when there are multiple identically named tests in different
>> libraries. If you were trying to solve the composite library name
>> issue ("algorithm" vs. "algorithm/string"), I suggest we first do the
>> Jamfile refactoring I talked about above; it would allow us to
>> simplify the code significantly, and then we can tackle this one.
>
> I don't think my patch is that bad. I agree with your point about
> multiple identically named tests. I was under the same impression.
>
> But process_jam_log can't actually deal with them, 

It definitely can, please see below.

> because it does
> around line 520:
>
> test2info.insert( std::make_pair( test_name, info ) );
>
> test2info is a map and test_name is the short name of test. 

Nope, it's not. It's the name extracted from one of the top lines of
the bjam log, and the lines corresponding to the "new" test cases do
contain the library name:

    boost-test(RUN) "utility/ref_test" : "libs\utility\ref_test.cpp"
    boost-test(RUN) "utility/addressof_test" : "libs\utility\addressof_test.cpp"
    ...

> There can only be one test with a given name in Boost. 

Not true, see "preprocessor/test/arithmetic.cpp" and
"mpl/test/arithmetic.cpp", for instance.

> So maybe we should replace it with something like:
>
> test2info.insert( std::make_pair( library_name + '/' + test_name, info ) );
>
> What do you think about it?

That's basically what is already there.

-- 
Aleksey Gurtovoy
MetaCommunications Engineering