From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Move tests of contrib/spi/ out of the core regression tests |
Date: | 2025-04-08 09:21:47 |
Message-ID: | 33bd7e6e-b652-46f2-a911-f0ba8909d060@iki.fi |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 08/04/2025 04:59, Tom Lane wrote:
> The attached patch removes test cases concerned with contrib/spi/
> from the core regression tests and instead puts them into new
> test files in contrib/spi/ itself.
+1
> My original motivation for looking at this was the discussion in
> [1] about whether to remove contrib/spi/refint.c entirely, since
> it's rather buggy and not a great example of our modern coding
> practices. So I wondered whether the core test cases that use it
> were contributing any significant amount of code coverage on the
> core code. (Spoiler: nope.) But I think this is generally good
> cleanup anyway, because it locates the test code for contrib/spi
> where a person would expect to find that, and removes some rather
> grotty coding in src/test/regress's Makefile and meson.build.
> As a side benefit, it removes some small number of cycles from
> the core tests, which seems like a good thing.
>
> The tests for the refint module are just moved over verbatim,
> except for using CREATE EXTENSION instead of manual declaration
> of the C functions. Also, I kept the tests for COMMENT ON TRIGGER
> in the core tests, by applying them to a different trigger.
>
> The tests for autoinc were kind of messy, because the behavior of
> autoinc itself was impossibly intertwined with the behavior of
> "ttdummy", which is an undocumented C function in regress.c.
> After some thought I decided we could just nuke ttdummy altogether,
> so the new autoinc.sql test is much simpler and more straightforward.
My only worry would if 'ttdummy' was a good showcase for how to write a
trigger function in C. But it's not a particularly good example. There
is a better example in the docs, and there's the 'autoinc' trigger too.
> This is too late for v18 of course, so I'll park it in the next CF.
In my opinion this would still be totally OK for v18. It's just test
changes. I would rather commit cleanups like this sooner than later.
--
Heikki Linnakangas
Neon (https://neon.tech)
From | Date | Subject | |
---|---|---|---|
Next Message | Thomas Munro | 2025-04-08 09:42:23 | Re: CREATE DATABASE with filesystem cloning |
Previous Message | Daniel Gustafsson | 2025-04-08 09:10:49 | pgsql: Add function to get memory context stats for processes |