On Dec 8, 2010, at 12:42 PM, Dimitri Fontaine wrote: > Kineticode Billing <da...@kineticode.com> writes: >> No, it's not. There are no unit tests at all. You can call the contrib >> modules and their tests acceptance tests, but that's not the same >> thing. > > Ok, I need some more guidance here. All contrib extension (there are 38 > of them) are using the CREATE EXTENSION command and checking the result > with the pg_regress framework. What are we missing?
unit tests. You add a bunch of functions. You need to test those functions. > I can see about adding DROP EXTENSION for all the tests, but that's > about it. If you add that, you'll also need something to CREATE EXTENSION with, eh? And also, tests to make sure that WITH SCHEMA works properly (however that shakes out). >> Okay, keep the installed control files. But don't make me distribute >> them unless absolutely necessary. > > Yes you have to distribute them, that's necessary. Sorry about that. I don't see why. Most of them are dead simple and could easily be Makefile variables. >> Sure. But you're mandating one version even if you have multiple >> extensions. That's the potentially confusing part. > > I see how confusing it is, because what you say ain't true. You can > always put different version numbers in the control file and even skip > the rule to produce the .control from the .control.in by providing the > .control directly. That's just a facility here. I see, okay. >> What? I don't follow what you're saying. > > You're complaining that a single EXTVERSION applied to more than one > extension's control file is confusing. What if we had EXTCOMMENT and > EXTRELOCATABLE in there too? What exactly are you expecting the Makefile > to look like? Mostly these will all have only one setting. In more complex cases perhaps one *would* be required to distribute a control file. >> In contrib. You seem to forget that there are a lot of third-party >> extensions out there already. > > True. That's still not the common case, and it's still covered the same > way as before, you need to restart to attach to shared memory. Okay. >> I would much rather retain that warning -- everyone should know about >> it -- and somehow convince SPI to be much less verbose in reporting >> issues. It should specify where the error came from (which query) and >> what the error actually is. > > The problem is much more complex than that and could well kill the patch > if we insist on fixing it as part of the extension's work, v1. The > problem is exposing more internals of the SQL parser into SPI so that > you can send a bunch of queries in an explicit way. Mind you, the > firsts version of the patch had something like that in there, but that > wouldn't have supported this use case. I've been told to simply use SPI > there. I agree that SPI should be fixed in a different project/patch. Go with what you've got, it will just highlight the problem with SPI more. Best, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
