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

From: Guillaume Melquiond (guillaume.melquiond_at_[hidden])
Date: 2004-07-10 03:50:51

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. 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 :).

> > 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.

> > Third, expected failures are not correctly shown in the advanced
> > regression logs [3] since some library names are wrong.
> 
> Consequently, this one is still open as well.
> 
> >
> > Fourth, some libraries are compiled in the wrong
> > directories. Instead of being compiled in
> > "bin/boost/libs/whatever/test/something.test", they are directly
> > compiled in "bin/boost/status/something.test" [4]. If you run the
> > regression tests, you will see what I mean.
> 
> I wouldn't say that they are "wrong", because that implies that this
> is a newly introduced behavior (read "bug"), and it's not the
> case. 
> 
> Basically, whether the test results go into "bin/boost/status" or
> "bin/boost/libs/" is determined by the age of the library: the new
> libraries that provide their own Jamfile for testing and use bjam's
> "subinclude" feature to bring it into "boost/status/jamfile" are put
> in "bin/boost/libs/<library-name>" location, and the libraries that
> are listing their test cases directly in the "status/jamfile" are put
> in "bin/boost/status".
> 
> Granted, the latter scheme is deficient and makes it unnecessarily
> hard to post-process the test results. How about we simply factor out
> these "inline" test-suites into the corresponding Jamfiles, and be
> done with the issue?

... Probably... If you say so... Sorry, I'm quite fond of the simplicity
of Makefiles, and whenever it comes to Jamfiles I'm a bit lost :-).

> > It's quite possible that the problems for the first three points are
> > related to the fourth.
> 
> The library name issue is not; it's simply a matter of tweaking
> "process_jam_log" to extract it correctly.
> 
> > Indeed, it seems only libraries wrongly compiled in
> > "bin/boost/status/something.test" get to be correctly identified.
> > It's the case for the "array" and "numeric/ublas" libraries for
> > example.
> >
> > I don't understand how regression testing takes place into
> > Boost. From my humble point of view, it is quite a mess.
> 
> The particular part that you've stumbled upon most definitely
> is, and it's about time to get it cleaned.
> 
> > I didn't understand why some libraries were compiled in the wrong
> > place; I suppose it has to do with bjam and their Jamfiles. I did
> > however find a way to correct the first two bugs (and probably the
> > third). The test_log.xml are indeed wrongly generated by
> > process_jam_log.
> >
> > So here is a patch to process_jam_log.cpp so that it correctly
> > generates the test_log.xml files. Unfortunately I don't understand
> > at all some parts of this program, so I only corrected what seemed
> > obviously wrong to me.
> 
> 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, 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. There can
only be one test with a given name in Boost. 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?

> > But maybe the bugs were somewhere else and I only designed a lame
> > workaround? Could the author of these lines shed some light on what
> > their purpose was supposed to be?
> 
> The purpose is to correctly match the test results with a
> corresponding test case from the right library, in presence of the
> discussed variability in the output directory structure. 

But it still works after my patch is applied, so...

> > In the meantime, maybe the regression testers could clear their
> >"bin" directory (so that all the test_log.xml are created from
> >scratch) and apply the patch? 
> 
> Due to the above, I don't think it's a good idea.
> 
> > Since Boost 1.31 is supposed to ship in less than 20 days, it would
> > be great if the regression logs were correctly generated.  
> 
> But this one is definitely worth fighting for. I'm going to look into
> the library name issue tomorrow.

I will write another patch so that test names are adjusted with their
library name (the bad one should be enough at first, it would remove the
first line change of my initial patch, I think) in test2info in
process_jam_log.

Regards,

Guillaume