Re: --compat mode behavior issues

[Martin Sebor <se...@roguewave.com>]

Andrew Black wrote:
[...snipped summary...]
> One (faulty) option would be to use the -O switch in compatability mode, 
> and provide it with a file such as /dev/stdout or /dev/fd/1.  On systems 
> where these 'magic' files exist, they reference the standard out file 
> descriptor.  However, the existence of such a file can't be relied on, 
> so we can't use them.  A similar magic file is /dev/tty, which is a file 
> descriptor pointing at the active console.  For the child process from 
> the exec utility, there is no associated console, so there is no place 
> to route the output, leading to a failure.

So this option is out.

> 
> A second method would be to alter the behavior of the test driver in 
> --compat mode to write to the .out file, and alter the exec utility to 
> redirect stderr and stdout to /dev/null in --compat mode.

Hmm, I'm not too wild about this one.

> 
> A final method would be to alter the rwtest library to treat the file 
> name of '-' when passed to the -O option as stdout, and make a similar 
> change to the RogueWave internal framework, then run the executables 
> with the options '--compat -O -'.  This would likely be less invasive 
> than the other option, and could be considered a useful enhancement.

This approach seems like the right way to go. I just changed
the Rogue Wave driver to recognize a single '-' as a shortcut
for stdout so you can do your thing in the stdcxx driver to
get it all working together.

Martin

Re: [PATCH] Re: --compat mode behavior issues

[Martin Sebor <se...@roguewave.com>]

Andrew Black wrote:
> Greetings Martin, all.
> 
> Attached is a patch that partly rewrites the check_compat_test method, 
> taking the points raised below into consideration.
> 
> Changelog:
>     * util/output.cpp (check_compat_test): Rewrite FSM to eliminate seek
>     to near end (was causing parsing issues on tests with output
>     following result block).

Looks okay except for the bool keyword. If we're still hoping
to someday compile the files with a C compiler we should avoid
using C++ keywords or constructs.

Martin

[PATCH] Re: --compat mode behavior issues

[Andrew Black <ab...@roguewave.com>]

Greetings Martin, all.

Attached is a patch that partly rewrites the check_compat_test method, 
taking the points raised below into consideration.

Changelog:
	* util/output.cpp (check_compat_test): Rewrite FSM to eliminate seek
	to near end (was causing parsing issues on tests with output
	following result block).

Martin Sebor wrote:
> Andrew Black wrote:
>> One probable reason is that some legacy tests (like 27_iosfile) 
>> produce multiple assertion totals (or are supposed to), and we want 
>> the last of these, rather than the first.
> 
> To handle this case we should process the file from the beginning
> and take the last total as the final one.
> 
>> A second possible reason was to make the parsing FSM slightly simpler 
>> (though adding an additional character or two to the pattern shouldn't 
>> make it much more complex).
> 
> I don't think avoiding the seek would add too much complexity
> (the default non-compat case doesn't seek and it's no more
> complex than the compat mode).
> 
>> A final possible reason was to make the parsing faster for long output 
>> files.
> 
> If the common case is the default mode (i.e., not --compat)
> we optimized the wrong branch :) In any case, getting correct
> results slowly is better than getting the wrong results fast.
> 
> I suggest you rewrite check_compat_test() to avoid seeking
> and scan the whole file, top to bottom, and take the last
> total.
> 
> Martin
> 

Re: --compat mode behavior issues

[Martin Sebor <se...@roguewave.com>]

Andrew Black wrote:
> One probable reason is that some legacy tests (like 27_iosfile) produce 
> multiple assertion totals (or are supposed to), and we want the last of 
> these, rather than the first.

To handle this case we should process the file from the beginning
and take the last total as the final one.

> A second possible reason was to make the 
> parsing FSM slightly simpler (though adding an additional character or 
> two to the pattern shouldn't make it much more complex).

I don't think avoiding the seek would add too much complexity
(the default non-compat case doesn't seek and it's no more
complex than the compat mode).

> A final 
> possible reason was to make the parsing faster for long output files.

If the common case is the default mode (i.e., not --compat)
we optimized the wrong branch :) In any case, getting correct
results slowly is better than getting the wrong results fast.

I suggest you rewrite check_compat_test() to avoid seeking
and scan the whole file, top to bottom, and take the last
total.

Martin

Re: --compat mode behavior issues

[Andrew Black <ab...@roguewave.com>]

One probable reason is that some legacy tests (like 27_iosfile) produce 
multiple assertion totals (or are supposed to), and we want the last of 
these, rather than the first.  A second possible reason was to make the 
parsing FSM slightly simpler (though adding an additional character or 
two to the pattern shouldn't make it much more complex).  A final 
possible reason was to make the parsing faster for long output files.

--Andrew Black

Martin Sebor wrote:
[...]
> 
> Why do we seek in compatibility mode instead of processing
> the file from top to bottom like we normally do? There could
> be any number of notes or other garbage at the end of the
> output file so seeking back is never going to be reliable.
> 
> Martin
> 

Re: --compat mode behavior issues

[Martin Sebor <se...@roguewave.com>]

Andrew Black wrote:
> Martin Sebor wrote:
>> Andrew Black wrote:
>> [...]
>>> Attached is a patch that implements the rwtest and makefile changes 
>>> for this option.  One problem observed is that the 
>>> 22.locale.time.get, 22.locale.time.put, and 27.objects now report a 
>>> FORMAT status.  The cause of these messages is output after the 
>>> assertion count has been printed.  The compatibility mode parsing 
>>> logic searches for the summary total in a different way than the 
>>> normal mode, and is therefore affected by this output where the 
>>> normal mode isn't.
>>
>> So is the problem that we start parsing too late in the file
>> (i.e., seek not far enough back) and end up skipping part of
>> the important output? Could we start parsing the output at
>> an earlier point in the file?
> 
> That would be an accurate assessment.  It would be possible to move the 
> seek position to a spot earlier in the file, but it would also be 
> necessary to alter the FSM used to search for the assertion count, as 
> the current method searches for the string "\n## ", which will also 
> match in the header of the file.  This spurious matching is what lead to 
> the change to output.cpp embedded into the patch.

Why do we seek in compatibility mode instead of processing
the file from top to bottom like we normally do? There could
be any number of notes or other garbage at the end of the
output file so seeking back is never going to be reliable.

Martin

Re: --compat mode behavior issues

[Andrew Black <ab...@roguewave.com>]

Martin Sebor wrote:
> Andrew Black wrote:
> [...]
>> Attached is a patch that implements the rwtest and makefile changes 
>> for this option.  One problem observed is that the 22.locale.time.get, 
>> 22.locale.time.put, and 27.objects now report a FORMAT status.  The 
>> cause of these messages is output after the assertion count has been 
>> printed.  The compatibility mode parsing logic searches for the 
>> summary total in a different way than the normal mode, and is 
>> therefore affected by this output where the normal mode isn't.
> 
> So is the problem that we start parsing too late in the file
> (i.e., seek not far enough back) and end up skipping part of
> the important output? Could we start parsing the output at
> an earlier point in the file?

That would be an accurate assessment.  It would be possible to move the 
seek position to a spot earlier in the file, but it would also be 
necessary to alter the FSM used to search for the assertion count, as 
the current method searches for the string "\n## ", which will also 
match in the header of the file.  This spurious matching is what lead to 
the change to output.cpp embedded into the patch.

> 
>>
>> --Andrew Black
>>
>> Changelog:
>>     * tests/src/driver.cpp (_rw_setopt_output_file): Add logic to 
>> treat the magic file name of '-' as a reference to stdout.
> 
> Please make sure the ChangeLog lines are 78 characters long
> or less.
> 
> Martin
> 
>>     * etc/config/GNUmakefile.tst (RUNFLAGS): Specify compatibility 
>> mode flags to enable parsing of legacy tests.
>>     * util/output.cpp (check_compat_test): Adjust seek position to 
>> avoid false format errors when an executable contains a very small 
>> number of tests.
>>
> 

Re: --compat mode behavior issues

[Martin Sebor <se...@roguewave.com>]

Andrew Black wrote:
[...]
> Attached is a patch that implements the rwtest and makefile changes for 
> this option.  One problem observed is that the 22.locale.time.get, 
> 22.locale.time.put, and 27.objects now report a FORMAT status.  The 
> cause of these messages is output after the assertion count has been 
> printed.  The compatibility mode parsing logic searches for the summary 
> total in a different way than the normal mode, and is therefore affected 
> by this output where the normal mode isn't.

So is the problem that we start parsing too late in the file
(i.e., seek not far enough back) and end up skipping part of
the important output? Could we start parsing the output at
an earlier point in the file?

> 
> --Andrew Black
> 
> Changelog:
>     * tests/src/driver.cpp (_rw_setopt_output_file): Add logic to treat 
> the magic file name of '-' as a reference to stdout.

Please make sure the ChangeLog lines are 78 characters long
or less.

Martin

>     * etc/config/GNUmakefile.tst (RUNFLAGS): Specify compatibility mode 
> flags to enable parsing of legacy tests.
>     * util/output.cpp (check_compat_test): Adjust seek position to avoid 
> false format errors when an executable contains a very small number of 
> tests.
> 

Re: --compat mode behavior issues

[Andrew Black <ab...@roguewave.com>]

Martin Sebor wrote:
> Andrew Black wrote:
[...]
>> A final method would be to alter the rwtest library to treat the file 
>> name of '-' when passed to the -O option as stdout, and make a similar 
>> change to the RogueWave internal framework, then run the executables 
>> with the options '--compat -O -'.  This would likely be less invasive 
>> than the other option, and could be considered a useful enhancement.
> 
> This approach seems like the right way to go. I just changed
> the Rogue Wave driver to recognize a single '-' as a shortcut
> for stdout so you can do your thing in the stdcxx driver to
> get it all working together.
> 
> Martin

Greetings all.

Attached is a patch that implements the rwtest and makefile changes for 
this option.  One problem observed is that the 22.locale.time.get, 
22.locale.time.put, and 27.objects now report a FORMAT status.  The 
cause of these messages is output after the assertion count has been 
printed.  The compatibility mode parsing logic searches for the summary 
total in a different way than the normal mode, and is therefore affected 
by this output where the normal mode isn't.

--Andrew Black

Changelog:
	* tests/src/driver.cpp (_rw_setopt_output_file): Add logic to treat the 
magic file name of '-' as a reference to stdout.
	* etc/config/GNUmakefile.tst (RUNFLAGS): Specify compatibility mode 
flags to enable parsing of legacy tests.
	* util/output.cpp (check_compat_test): Adjust seek position to avoid 
false format errors when an executable contains a very small number of 
tests.