Hi All!
I am about to set "Ready for commit" status to this patch. So there is my
summary for commiter, so one does not need to carefully read all the thread.
This patch is consists of three parts. May be they should be commited
separately, then Fabien will split them, I think.
1. The tests.
Tests are quite good. May be I myself would write them in another style, but
"there is more than one way to do it" in perl, you know.
These test covers more than 90% of C code of pgbench, which is good. (I did
not check this myself, but see no reason not to trust Fabien)
The most doubtful part of the patch is the following: the patch introduce
command_checks_all function in TestLib.pm that works like command_like
function but also allows to check STDERR output and exit status.
First: I have some problem with the name, I would call it command_like_more or
something similar, but I decided to leave it for commiter to make final choice.
Second: I think that command_checks and command_like do very similar things. I
think that later all lests should be rewritten to use command_checks, and get
rid of command_like, And I do not know how to put this better in the
developing workflow. May be it should be another patch after this one is
commited.
2. Patch for -t/-R/-L case.
Current pgbench does not process -t/-R/-L case correctly. This was also fixed
in this patch.
Now if you set number of transactions using -t/--transactions, combining with
-R/--rate or -L/--latency-limit, you can be sure there would be not more than
were specified in -t and they are properly counted.
This part is quite clear
3. \n process in process_backslash_command error output
process_backslash_command raises an error if there are some problems with
backslash commands, and prints the command that has error.
But it considers that there is always \n symbol at the end of the command and
just chop it out. But when the backslash command is at the end of the file,
there is no \n at the end of line.
So this patch introduces expr_scanner_chomp_substring function that works just
like expr_scanner_get_substring but it skips \n or \r\n symbols at the end of
line.
I still have some problems with function name here, but have no idea how to do
it better.
So that's it. I hope my involvement in the review process were useful. Will be
happy to see this patch commited into master :-)
--
Do code for fun. Can do it for money (Perl & C/C++ ~10h/week)
