Re: Contribution to Perldoc for TestLib module in Postgres
On Mon, Sep 02, 2019 at 01:48:14PM -0400, Alvaro Herrera wrote: > Agreed ... that's pretty much the same thing I tried to say upthread. I > wrote my own synopsis, partly using your suggestions. I reworded the > description for the routines (I don't think there's any I didn't touch), > added a mention of $windows_os, added a =head1 to split out the ad-hoc > routines from the Test::More wrappers. > > And pushed. > > Please give it another look. It might need more polish. Thanks for committing. I have read through the commit and I am not noticing any issue sorting out. One thing may be to give a short description for some of the routine's arguments like check_mode_recursive, but I think that we could live without that either. -- Michael signature.asc Description: PGP signature
Re: Contribution to Perldoc for TestLib module in Postgres
On Tue, Jul 30, 2019 at 4:47 PM Michael Paquier wrote: > On Fri, Jul 26, 2019 at 09:51:34AM -0400, Andrew Dunstan wrote: > > I've fixed the bitrot and some other infelicities on this patch. It's > > not commitable yet but I think it's more reviewable. > > Thanks, I had a look at this version. > > [review listing things to fix] Hi Ram, Based on the above, it sounds like we want this patch but it needs a bit more work. It's now the end of CF1. I'm moving this one to CF2 (September). Please post a new patch when ready. Thanks! -- Thomas Munro https://enterprisedb.com
Re: Contribution to Perldoc for TestLib module in Postgres
On Fri, Jul 26, 2019 at 09:51:34AM -0400, Andrew Dunstan wrote:
> I've fixed the bitrot and some other infelicities on this patch. It's
> not commitable yet but I think it's more reviewable.
Thanks, I had a look at this version.
+ # Returns the real directory for a virtual path directory under msys
+ real_dir(dir)
real_dir() is no more.
perl2host() is missing.
+ #TODO
+ command_like_safe(cmd, expected_stdout, test_name)
[...]
+=pod
+
+=item command_like_safe(cmd, expected_stdout, test_name)
+
+TODO
+
+=cut
Update not to miss.
+Runs the command which is passed as argument to the function. On failure it
+abandons further tests and exits the program.
"On failure the test suite exits immediately."
I think that the SYNOPSIS could be shaped better. As of now it is a
simple succession of the same commands listed without any link to each
other, which is contrary for example to what we do in PostgresNode.pm,
where it shows up a set of small examples which are useful to
understand how to shape the tests and the possible interactions
between the routines of the module. My take would be to keep it
simple and minimal as TestLib.pm is the lowest level of our TAP test
infrastructure. So here are some simple suggestions, and we could go
with this set to begin with:
# Test basic output of a command.
program_help_ok('initdb');
program_version_ok('initdb');
program_options_handling_ok('initdb');
# Test option combinations
command_fails(['initdb', '--invalid-option'],
'command fails with invalid option');
my $tempdir = TestLib::tempdir;
command_ok('initdb', '-D', $tempdir);
Another thing is that the examples should not overlap with what
PostgresNode.pm presents, and that it is not necessary to show up all
the routines. It also makes little sense to describe in the synopsis
the routines in a way which duplicates with the descriptions on top of
each routine.
--
Michael
signature.asc
Description: PGP signature
Re: Contribution to Perldoc for TestLib module in Postgres
On 7/10/19 9:38 AM, Alvaro Herrera wrote: > On 2019-Apr-11, Iwata, Aya wrote: > >> In the above document, why not write a description after the function name? >> I think it is better to write the function name first and then the >> description. >> In your code; >> #Checks if all the tests passed or not >> all_tests_passing() >> >> In my suggestion; >> all_tests_passing() >> Checks if all the tests passed or not > Yeah, so there are two parts in the submitted patch: first the synopsis > list the methods using this format you describe, and later the METHODS > section lists then again, using your suggested style. I think we should > do away with the long synopsis -- maybe keep it as just the "use > TestLib" line, and then let the METHODS section list and describe the > methods. > >> And some functions return value. How about adding return information >> to the above doc? > That's already in the METHODS section for some of them. For example: > > all_tests_passing() > Returns 1 if all the tests pass. Otherwise returns 0 > > It's missing for others, such as "tempdir". > > In slurp_file you have this: > Opens the file provided as an argument to the function in read mode(as > indicated by <). > I think the parenthical comment is useless; remove that. > > Please break long source lines (say to 78 chars -- make sure pgperltidy > agrees), and keep some spaces after sentence-ending periods and other > punctuation. > I've fixed the bitrot and some other infelicities on this patch. It's not commitable yet but I think it's more reviewable. cheers andrew -- Andrew Dunstanhttps://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm index 6195c21c59..ebb0eb82e7 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -1,9 +1,108 @@ -# TestLib, low-level routines and actions regression tests. -# -# This module contains a set of routines dedicated to environment setup for -# a PostgreSQL regression test run and includes some low-level routines -# aimed at controlling command execution, logging and test functions. This -# module should never depend on any other PostgreSQL regression test modules. + +=pod + +=head1 NAME + +TestLib - module containing routines for environment setup, low-level routines +for command execution, logging and test functions + +=head1 SYNOPSIS + + use TestLib; + + # Checks if all the tests passed or not + all_tests_passing() + + # Creates a temporary directory + tempdir(prefix) + + # Creates a temporary directory outside the build tree for the + # Unix-domain socket + tempdir_short() + + # Returns the real directory for a virtual path directory under msys + real_dir(dir) + + # Runs the command which is passed as an argument + system_log() + + # Runs the command which is passed as an argument. On failure abandons + # all other tests + system_or_bail() + + # Runs the command which is passed as an argument. + run_log() + + # Returns the output after running a command + run_command(dir) + + # Generate a string made of the given range of ASCII characters + generate_ascii_string(from_char, to_char) + + # Read the contents of the directory + slurp_dir(dir) + + # Read the contents of the file + slurp_file(filename) + + # Appends a string at the end of a given file + append_to_file(filename, str) + + # Check that all file/dir modes in a directory match the expected values + check_mode_recursive(dir, expected_dir_mode, expected_file_mode, + ignore_list) + + # Change mode recursively on a directory + chmod_recursive(dir, dir_mode, file_mode) + + # Check presence of a given regexp within pg_config.h + check_pg_config(regexp) + + # Test function to check that the command runs successfully + command_ok(cmd, test_name) + + # Test function to check that the command fails + command_fails(cmd, test_name) + + # Test function to check that the command exit code matches the + # expected exit code + command_exit_is(cmd, expected, test_name) + + # Test function to check that the command supports --help option + program_help_ok(cmd) + + # Test function to check that the command supports --version option + program_version_ok(cmd) + + # Test function to check that a command with invalid option returns + # non-zero exit code and error message + program_options_handling_ok(cmd) + + # Test function to check that the command runs successfully and the + # output matches the given regular expression + command_like(cmd, expected_stdout, test_name) + + #TODO + command_like_safe(cmd, expected_stdout, test_name) + + # Test function to check that the command fails and the error message + # matches the given regular expression + command_fails_like(cmd, expected_stderr, test_name) + + # Test function to run a command and check its status and outputs + command_checks_all(cmd, expected_ret, out,
Re: Contribution to Perldoc for TestLib module in Postgres
On 2019-Apr-11, Iwata, Aya wrote: > In the above document, why not write a description after the function name? > I think it is better to write the function name first and then the > description. > In your code; > #Checks if all the tests passed or not > all_tests_passing() > > In my suggestion; > all_tests_passing() > Checks if all the tests passed or not Yeah, so there are two parts in the submitted patch: first the synopsis list the methods using this format you describe, and later the METHODS section lists then again, using your suggested style. I think we should do away with the long synopsis -- maybe keep it as just the "use TestLib" line, and then let the METHODS section list and describe the methods. > And some functions return value. How about adding return information > to the above doc? That's already in the METHODS section for some of them. For example: all_tests_passing() Returns 1 if all the tests pass. Otherwise returns 0 It's missing for others, such as "tempdir". In slurp_file you have this: Opens the file provided as an argument to the function in read mode(as indicated by <). I think the parenthical comment is useless; remove that. Please break long source lines (say to 78 chars -- make sure pgperltidy agrees), and keep some spaces after sentence-ending periods and other punctuation. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: Contribution to Perldoc for TestLib module in Postgres
On Tue, Jul 09, 2019 at 03:16:01PM +0200, Daniel Gustafsson wrote: > The v2 patch is somewhat confused as it has Windows carriage returns rather > than newlines, so it replaces the entire file making the diff hard to read. > It > also includes a copy of TestLib and the v1 patch and has a lot of whitespace > noise. Nobody can provide a clear review if the files are just fully rewritten even based on a read of the patch. Perhaps you are working on Windows and forgot to configure core.autocrlf with "git config". That could make your life easier. I have switched the patch as "waiting on author" for now. -- Michael signature.asc Description: PGP signature
Re: Contribution to Perldoc for TestLib module in Postgres
> On 7 Apr 2019, at 20:04, Ramanarayana wrote: > Please find the updated patch. Added to the commitfest as well The v2 patch is somewhat confused as it has Windows carriage returns rather than newlines, so it replaces the entire file making the diff hard to read. It also includes a copy of TestLib and the v1 patch and has a lot of whitespace noise. Please redo the patch on a clean tree to get a more easily digestable patch. cheers ./daniel
RE: Contribution to Perldoc for TestLib module in Postgres
Hi Ram, I think this documentation helps people who want to understand functions. >Please find the updated patch. Added to the commitfest as well I have some comments. I think some users who would like to know custom function check src/test/perl/README at first. How about add comments to the README file, such as "See Testlib.pm if you want to know more details"? In the above document, why not write a description after the function name? I think it is better to write the function name first and then the description. In your code; #Checks if all the tests passed or not all_tests_passing() In my suggestion; all_tests_passing() Checks if all the tests passed or not And some functions return value. How about adding return information to the above doc? I find ^M character in your new code. Please use LF line ending not CRLF or get rid of it in next patch. Regards, Aya Iwata
Re: Contribution to Perldoc for TestLib module in Postgres
Hi, Please find the updated patch. Added to the commitfest as well Regards, Ram. v2_perldoc_testlib.patch.patch Description: Binary data
Re: Contribution to Perldoc for TestLib module in Postgres
On Fri, Mar 22, 2019 at 04:59:58PM +0530, Ramanarayana wrote: > Please find the first version of the patch for review. I was not sure what > some of the functions are used for and marked them with TODO. This is only adding some documentation to an internal perl module we ship, so it is far from being a critical part and we could still get that into v12, still there are many patches waiting for integration into v12 and this has showed up very late. Could you please register this patch to the commit fest once you have a patch you think is fit for merging? Here is the next commit fest link: https://commitfest.postgresql.org/23/ -- Michael signature.asc Description: PGP signature
Re: Contribution to Perldoc for TestLib module in Postgres
Hi, Please find the first version of the patch for review. I was not sure what some of the functions are used for and marked them with TODO. Cheers Ram 4.0 v1_perldoc_testlib.patch Description: Binary data
Re: Contribution to Perldoc for TestLib module in Postgres
Sure, please go ahead. Regards, Prajwal. On Thu, 21 Mar 2019, 19:11 Ramanarayana, wrote: > Hi, > Can I take this up? > > Regards, > Ram >
Re: Contribution to Perldoc for TestLib module in Postgres
Hi, Can I take this up? Regards, Ram
Re: Contribution to Perldoc for TestLib module in Postgres
On Tue, Mar 19, 2019 at 09:05:29AM -0300, Alvaro Herrera wrote: > Yes, it is, please do. +1. -- Michael signature.asc Description: PGP signature
Re: Contribution to Perldoc for TestLib module in Postgres
On 2019-Mar-19, Prajwal A V wrote: > I could not find the perldoc for TestLib module in src/test/perl and found > some difficultly in understanding what each function does while other > modules in the src/test folder had perldoc and it was easy to understand > the functions. > > I would like to contribute for the perldoc for TestLib. I am looking for > your suggestions if this contribution is worth doing. Yes, it is, please do. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Contribution to Perldoc for TestLib module in Postgres
Hi Hackers, I was going through the regression testing framework used in postgres. I was trying to understand the custom functions written in perl for postgres. I could not find the perldoc for TestLib module in src/test/perl and found some difficultly in understanding what each function does while other modules in the src/test folder had perldoc and it was easy to understand the functions. I would like to contribute for the perldoc for TestLib. I am looking for your suggestions if this contribution is worth doing. Regards, Prajwal
