From: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
---|---|
To: | Noah Misch <noah(at)leadboat(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, "David E(dot) Wheeler" <david(at)justatheory(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Make more portable TAP tests of initdb |
Date: | 2015-05-03 02:15:40 |
Message-ID: | CAB7nPqRCTrfDmJXvwmmOh_YLmFU9UKAziJoLYL1JotF6_p5eHw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, May 3, 2015 at 8:09 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> On Fri, May 01, 2015 at 03:23:44PM +0900, Michael Paquier wrote:
>> On Thu, Apr 30, 2015 at 12:17 PM, Noah Misch <noah(at)leadboat(dot)com> wrote:
>> > As I pondered this, I felt it would do better to solve a different problem.
>> > The "rm -rf" invocations presumably crept in to reduce peak disk usage.
>> > Considering the relatively-high duration of a successful initdb run, I doubt
>> > we get good value from so many positive tests. I squashed those positive
>> > tests into one, as attached. (I changed the order of negative tests but kept
>> > them all.) This removed the need for intra-script cleaning, and it
>> > accelerated script runtime from 22s to 9s. Does this seem good to you?
>>
>> That's a fight code simplicity vs. test performance. FWIW, I like the
>> test suite with many positive tests because it is easy to read the
>> test flow. And as this test suite is meant to grow up with new tests
>> in the future, I am guessing that people may not follow the
>> restriction this patch adds all the time.
>
> True.
>
>> command_fails(
>> - [ 'initdb', "$tempdir/data", '-X', 'pgxlog' ],
>> + [ 'initdb', $datadir, '-X', 'pgxlog' ],
>> 'relative xlog directory not allowed');
>> $datadir should be the last argument. This would break on platforms
>> where src/port/getopt_long.c is used, like Windows.
>
> I committed that as a separate patch; thanks.
>
>> +command_ok([ 'initdb', '-N', '-T', 'german', '-X', $xlogdir, $datadir ],
>> + 'successful creation');
>> This is grouping the test for nosync, existing nonempty xlog
>> directory, and the one for selection of the default dictionary.
>> Shouldn't this test name be updated otherwise then?
>>
>> Could you add a comment mentioning that tests are grouped to reduce
>> I/O impact during a run?
>
> Committed with a comment added. The "successful creation" test name is
> generic because it's the designated place to add testing of new options.
Thanks. The overall result looks good to me.
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Joe Wildish | 2015-05-03 09:24:46 | Re: Implementing SQL ASSERTION |
Previous Message | Shigeru HANADA | 2015-05-03 01:51:13 | Re: postgres_fdw join pushdown (was Re: Custom/Foreign-Join-APIs) |