Horton

Thread

Commits

  1. Remove more unused module imports from TAP tests

  2. Remove unused module imports from TAP tests

  1. Small TAP tests cleanup for Windows and unused modules

    Daniel Gustafsson <daniel@yesql.se> — 2022-02-16T21:36:24Z

    Seeing msys in TAP tests mentioned in a thread [1] tonight reminded me about
two related (well, one of them) patches I had sitting around, so rather than
forgetting again here are some small cleanups.

0001 attempts to streamline how we detect Windows in the TAP tests (after that
there is a single msys check left that I'm not sure about, but [1] seems to
imply it could go); 0002 removes some unused module includes which either were
used at some point in the past or likely came from copy/paste.

--
Daniel Gustafsson		https://vmware.com/

[1] https://postgr.es/m/0ba775a2-8aa0-0d56-d780-69427cf6f33d@dunslane.net

  2. Re: Small TAP tests cleanup for Windows and unused modules

    Andrew Dunstan <andrew@dunslane.net> — 2022-02-16T22:04:01Z

    On 2/16/22 16:36, Daniel Gustafsson wrote:
> Seeing msys in TAP tests mentioned in a thread [1] tonight reminded me about
> two related (well, one of them) patches I had sitting around, so rather than
> forgetting again here are some small cleanups.
>
> 0001 attempts to streamline how we detect Windows in the TAP tests (after that
> there is a single msys check left that I'm not sure about, but [1] seems to
> imply it could go); 0002 removes some unused module includes which either were
> used at some point in the past or likely came from copy/paste.
>

0002 looks OK at first glance.


0001 is something we should investigate. It's really going in the wrong
direction I suspect. We should be looking to narrow the scope of these
platform-specific bits of processing, not expand them.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  3. Re: Small TAP tests cleanup for Windows and unused modules

    Daniel Gustafsson <daniel@yesql.se> — 2022-02-16T22:29:21Z

    > On 16 Feb 2022, at 23:04, Andrew Dunstan <andrew@dunslane.net> wrote:
> 
> On 2/16/22 16:36, Daniel Gustafsson wrote:
>> Seeing msys in TAP tests mentioned in a thread [1] tonight reminded me about
>> two related (well, one of them) patches I had sitting around, so rather than
>> forgetting again here are some small cleanups.
>> 
>> 0001 attempts to streamline how we detect Windows in the TAP tests (after that
>> there is a single msys check left that I'm not sure about, but [1] seems to
>> imply it could go); 0002 removes some unused module includes which either were
>> used at some point in the past or likely came from copy/paste.
>> 
> 
> 0002 looks OK at first glance.

Thanks!

> 0001 is something we should investigate. It's really going in the wrong
> direction I suspect. We should be looking to narrow the scope of these
> platform-specific bits of processing, not expand them.

No arguments there, I was only looking at making sure that we detect the OS in
the same way everywhere, but if you're working towards eliminating this there
is little use in changing anything.

--
Daniel Gustafsson		https://vmware.com/

  4. Re: Small TAP tests cleanup for Windows and unused modules

    Andrew Dunstan <andrew@dunslane.net> — 2022-02-16T22:36:14Z

    On 2/16/22 17:04, Andrew Dunstan wrote:
> On 2/16/22 16:36, Daniel Gustafsson wrote:
>> Seeing msys in TAP tests mentioned in a thread [1] tonight reminded me about
>> two related (well, one of them) patches I had sitting around, so rather than
>> forgetting again here are some small cleanups.
>>
>> 0001 attempts to streamline how we detect Windows in the TAP tests (after that
>> there is a single msys check left that I'm not sure about, but [1] seems to
>> imply it could go); 0002 removes some unused module includes which either were
>> used at some point in the past or likely came from copy/paste.
>>
> 0002 looks OK at first glance.
>
>
> 0001 is something we should investigate. It's really going in the wrong
> direction I suspect. We should be looking to narrow the scope of these
> platform-specific bits of processing, not expand them.
>
>


More specifically, all the tests in question are now passing on jacana
and fairywren, and their $Config{osname} is NOT 'msys' (it's 'MSWin32').

So we should simply remove any line that ends "if $Config{osname} eq
'msys';" since we don't have any such animals any more.


cheers


andrew


--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  5. Re: Small TAP tests cleanup for Windows and unused modules

    Andres Freund <andres@anarazel.de> — 2022-02-16T22:54:14Z

    Hi, 

On February 16, 2022 2:04:01 PM PST, Andrew Dunstan <andrew@dunslane.net> wrote:
>
>On 2/16/22 16:36, Daniel Gustafsson wrote:
>> Seeing msys in TAP tests mentioned in a thread [1] tonight reminded me about
>> two related (well, one of them) patches I had sitting around, so rather than
>> forgetting again here are some small cleanups.
>>
>> 0001 attempts to streamline how we detect Windows in the TAP tests (after that
>> there is a single msys check left that I'm not sure about, but [1] seems to
>> imply it could go); 0002 removes some unused module includes which either were
>> used at some point in the past or likely came from copy/paste.
>>
>
>0002 looks OK at first glance.
>
>
>0001 is something we should investigate. It's really going in the wrong
>direction I suspect. We should be looking to narrow the scope of these
>platform-specific bits of processing, not expand them.

Yes. The vast majority of those skips looks frankly irresponsible, indicating bugs in the test or postgres. There's definitely valid cases (e.g. testing sysv shm), but most don't look valid to me.

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

  6. Re: Small TAP tests cleanup for Windows and unused modules

    Michael Paquier <michael@paquier.xyz> — 2022-02-17T01:38:00Z

    On Wed, Feb 16, 2022 at 05:36:14PM -0500, Andrew Dunstan wrote:
> More specifically, all the tests in question are now passing on jacana
> and fairywren, and their $Config{osname} is NOT 'msys' (it's 'MSWin32').

Indeed.  0001 is incorrect.

> So we should simply remove any line that ends "if $Config{osname} eq
> 'msys';" since we don't have any such animals any more.

So, that's related to [1], meaning that we don't have any buildfarm
members that run the perl flavor from Msys.  And, as a result, we
don't need to worry about the CRLF conversions in any of the perl
modules?  That would be nice.

[1]: https://www.postgresql.org/message-id/0ba775a2-8aa0-0d56-d780-69427cf6f33d@dunslane.net 
--
Michael

  7. Re: Small TAP tests cleanup for Windows and unused modules

    Daniel Gustafsson <daniel@yesql.se> — 2022-02-18T21:02:23Z

    > On 16 Feb 2022, at 23:36, Andrew Dunstan <andrew@dunslane.net> wrote:
> On 2/16/22 17:04, Andrew Dunstan wrote:
>> On 2/16/22 16:36, Daniel Gustafsson wrote:
>>> Seeing msys in TAP tests mentioned in a thread [1] tonight reminded me about
>>> two related (well, one of them) patches I had sitting around, so rather than
>>> forgetting again here are some small cleanups.
>>> 
>>> 0001 attempts to streamline how we detect Windows in the TAP tests (after that
>>> there is a single msys check left that I'm not sure about, but [1] seems to
>>> imply it could go); 0002 removes some unused module includes which either were
>>> used at some point in the past or likely came from copy/paste.
>>> 
>> 0002 looks OK at first glance.
>> 
>> 0001 is something we should investigate. It's really going in the wrong
>> direction I suspect. We should be looking to narrow the scope of these
>> platform-specific bits of processing, not expand them.
> 
> More specifically, all the tests in question are now passing on jacana
> and fairywren, and their $Config{osname} is NOT 'msys' (it's 'MSWin32').

+1

> So we should simply remove any line that ends "if $Config{osname} eq
> 'msys';" since we don't have any such animals any more.

Since msys is discussed in the other thread let's drop the 0001 from this
thread and just go ahead with 0002.

--
Daniel Gustafsson		https://vmware.com/

  8. Re: Small TAP tests cleanup for Windows and unused modules

    Daniel Gustafsson <daniel@yesql.se> — 2022-03-24T20:02:09Z

    > On 18 Feb 2022, at 22:02, Daniel Gustafsson <daniel@yesql.se> wrote:

> .. let's drop the 0001 from this thread and just go ahead with 0002.

I applied the 0002 patch today, cleaning up the unused module imports.

--
Daniel Gustafsson		https://vmware.com/

  9. Re: Small TAP tests cleanup for Windows and unused modules

    Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2022-03-25T12:42:18Z

    Daniel Gustafsson <daniel@yesql.se> writes:

>> On 18 Feb 2022, at 22:02, Daniel Gustafsson <daniel@yesql.se> wrote:
>
>> .. let's drop the 0001 from this thread and just go ahead with 0002.
>
> I applied the 0002 patch today, cleaning up the unused module imports.

A quick git grep¹ revealed a few more extraneous `use Config;`
statements (and manual inspection a few more unused modules in one
file). Here's a patch that removes those. It passes tests using

    ./configure --enable-tap-tests --with-ldap
    make check-world PG_TEST_EXTRA=ldap

- ilmari

[1] git grep -L '[%@$]Config\b' $(git grep -l 'use Config')

  10. Re: Small TAP tests cleanup for Windows and unused modules

    Daniel Gustafsson <daniel@yesql.se> — 2022-03-26T23:20:44Z

    > On 25 Mar 2022, at 13:42, Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:

> A quick git grep¹ revealed a few more extraneous `use Config;`
> statements (and manual inspection a few more unused modules in one
> file).

Thanks for digging these up, they look correct to me.

> src/bin/pg_ctl/t/001_start_stop.pl | 3 ---
> src/bin/pg_rewind/t/RewindTest.pm  | 1 -

These Config references were removed in 1c6d46293.  Fcntl ':mode' and
File::stat were added in c37b3d08c which was probably a leftover from an
earlier version of the patch, as the function using these was added to
PostgresNode in that commit.

> src/test/ldap/t/001_auth.pl        | 1 -

The Config reference here was added in ee56c3b21 which in turn use $^O instead
of interrogating Config, so it can be removed as well.

I'll take it for a tour on the CI and will then apply.

--
Daniel Gustafsson		https://vmware.com/

  11. Re: Small TAP tests cleanup for Windows and unused modules

    Daniel Gustafsson <daniel@yesql.se> — 2022-03-27T20:44:23Z

    > On 27 Mar 2022, at 00:20, Daniel Gustafsson <daniel@yesql.se> wrote:

> I'll take it for a tour on the CI and will then apply.

Which is now done, thanks!

--
Daniel Gustafsson		https://vmware.com/