Обсуждение: Re: [COMMITTERS] pgsql: Add TAP tests for pg_dump
On Fri, May 6, 2016 at 4:07 PM, Stephen Frost <sfrost@snowman.net> wrote: > * Tom Lane (tgl@sss.pgh.pa.us) wrote: >> Stephen Frost <sfrost@snowman.net> writes: >> > * Stephen Frost (sfrost@snowman.net) wrote: >> >> Looks like the test_pg_dump extension made the Windows builds upset. >> >> I'm guessing that's because I set 'MODULES_big' even though there isn't >> >> a .c component. >> >> >> >> Doing a local build with that commented out, assuming that works and >> >> doesn't generate the .so any more on my Linux box, I'll push the change >> >> up to hopefully fix those buildfarm members. >> >> > Alright, apparently that made other Windows buildfarm members unhappy... >> >> > I guess the next approach will be to add back MODULES_big and add in a >> > .c file for the Windows systems to be happy about. I'm certainly open >> > to other suggestions. >> >> You should not need to do that; cf src/test/modules/test_extensions, >> which has got SQL-only extensions. > > test_extensions is also included in the "contrib_excludes" list in > src/tools/msvc/Mkvcbuild.pm that I mentioned in my last email, so I'm > thinking that's what is needed. > >> But at this point I think Peter's complaint has some force to it, and that >> what you ought to do is revert the testing patch. You can have another go >> after beta1. > > Are you suggesting commiting to still-9.6-HEAD post-beta1? I took > Peter's comment as suggesting that adding the tests would have to wait > til after we branched 9.6, as we do for features. > > I'd really like to have these tests included as that will make them > available to others more easily to add on to, and I'm certainly planning > to continue adding tests until I get pg_dump.c's coverage a lot better. > That seems like the perfect kind of effort that should be happening > right now- adding more tests and working to make sure that what's been > committed is correct (and fixing it when it isn't, as discovered by the > test suite with transforms and casts...). I think what he's suggesting right now is that you revert the patch that is turning the BF red right before beta. We can iron out what else to do later. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
* Robert Haas (robertmhaas@gmail.com) wrote: > On Fri, May 6, 2016 at 4:07 PM, Stephen Frost <sfrost@snowman.net> wrote: > > * Tom Lane (tgl@sss.pgh.pa.us) wrote: > >> Stephen Frost <sfrost@snowman.net> writes: > >> > * Stephen Frost (sfrost@snowman.net) wrote: > >> >> Looks like the test_pg_dump extension made the Windows builds upset. > >> >> I'm guessing that's because I set 'MODULES_big' even though there isn't > >> >> a .c component. > >> >> > >> >> Doing a local build with that commented out, assuming that works and > >> >> doesn't generate the .so any more on my Linux box, I'll push the change > >> >> up to hopefully fix those buildfarm members. > >> > >> > Alright, apparently that made other Windows buildfarm members unhappy... > >> > >> > I guess the next approach will be to add back MODULES_big and add in a > >> > .c file for the Windows systems to be happy about. I'm certainly open > >> > to other suggestions. > >> > >> You should not need to do that; cf src/test/modules/test_extensions, > >> which has got SQL-only extensions. > > > > test_extensions is also included in the "contrib_excludes" list in > > src/tools/msvc/Mkvcbuild.pm that I mentioned in my last email, so I'm > > thinking that's what is needed. > > > >> But at this point I think Peter's complaint has some force to it, and that > >> what you ought to do is revert the testing patch. You can have another go > >> after beta1. > > > > Are you suggesting commiting to still-9.6-HEAD post-beta1? I took > > Peter's comment as suggesting that adding the tests would have to wait > > til after we branched 9.6, as we do for features. > > > > I'd really like to have these tests included as that will make them > > available to others more easily to add on to, and I'm certainly planning > > to continue adding tests until I get pg_dump.c's coverage a lot better. > > That seems like the perfect kind of effort that should be happening > > right now- adding more tests and working to make sure that what's been > > committed is correct (and fixing it when it isn't, as discovered by the > > test suite with transforms and casts...). > > I think what he's suggesting right now is that you revert the patch > that is turning the BF red right before beta. We can iron out what > else to do later. Alright, I'll revert the TAP tests and we can discuss what to do next post-beta1. At least a few runs got in and looked clean, other than the Windows issue with the extension building. Thanks! Stephen
Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, May 6, 2016 at 4:07 PM, Stephen Frost <sfrost@snowman.net> wrote:
>> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>>> But at this point I think Peter's complaint has some force to it, and that
>>> what you ought to do is revert the testing patch. You can have another go
>>> after beta1.
>> Are you suggesting commiting to still-9.6-HEAD post-beta1? I took
>> Peter's comment as suggesting that adding the tests would have to wait
>> til after we branched 9.6, as we do for features.
>>
>> I'd really like to have these tests included as that will make them
>> available to others more easily to add on to, and I'm certainly planning
>> to continue adding tests until I get pg_dump.c's coverage a lot better.
>> That seems like the perfect kind of effort that should be happening
>> right now- adding more tests and working to make sure that what's been
>> committed is correct (and fixing it when it isn't, as discovered by the
>> test suite with transforms and casts...).
> I think what he's suggesting right now is that you revert the patch
> that is turning the BF red right before beta. We can iron out what
> else to do later.
Yes. I have no objection to adding more test cases post-beta1, but
I'd like to have the buildfarm green through the weekend. This isn't
the best time to be debugging-by-buildfarm.
If you like, you can try the @contrib_excludes addition that was mentioned
before and see if that fixes it. But if it doesn't, it's time to cut our
losses.
regards, tom lane
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > Robert Haas <robertmhaas@gmail.com> writes: > > On Fri, May 6, 2016 at 4:07 PM, Stephen Frost <sfrost@snowman.net> wrote: > >> * Tom Lane (tgl@sss.pgh.pa.us) wrote: > >>> But at this point I think Peter's complaint has some force to it, and that > >>> what you ought to do is revert the testing patch. You can have another go > >>> after beta1. > > >> Are you suggesting commiting to still-9.6-HEAD post-beta1? I took > >> Peter's comment as suggesting that adding the tests would have to wait > >> til after we branched 9.6, as we do for features. > >> > >> I'd really like to have these tests included as that will make them > >> available to others more easily to add on to, and I'm certainly planning > >> to continue adding tests until I get pg_dump.c's coverage a lot better. > >> That seems like the perfect kind of effort that should be happening > >> right now- adding more tests and working to make sure that what's been > >> committed is correct (and fixing it when it isn't, as discovered by the > >> test suite with transforms and casts...). > > > I think what he's suggesting right now is that you revert the patch > > that is turning the BF red right before beta. We can iron out what > > else to do later. > > Yes. I have no objection to adding more test cases post-beta1, but > I'd like to have the buildfarm green through the weekend. This isn't > the best time to be debugging-by-buildfarm. Understood. > If you like, you can try the @contrib_excludes addition that was mentioned > before and see if that fixes it. But if it doesn't, it's time to cut our > losses. Ok, I like that quite a bit better and will give it a shot, but if it doesn't work, I'll revert it immediately. Thanks! Stephen
Tom, all, * Tom Lane (tgl@sss.pgh.pa.us) wrote: > If you like, you can try the @contrib_excludes addition that was mentioned > before and see if that fixes it. But if it doesn't, it's time to cut our > losses. Alright, it certainly *appears* to be working. All of the Windows buildfarm animals which have reported back since I added test_pg_dump to that list have been green. There are still a few boxes outstanding, but that seems like a good sign too- they failed pretty quickly before. I'm saying all this because I have to step out for a couple of hours. I'll keep an eye on it and will be around later tonight to revert the tests if necessary, but I'm not going to complain if someone else chooses to do so if those last few Windows boxes come back red again before I'm around again. Thanks! Stephen