Hii,
On Tue, Jan 6, 2026 at 5:49 PM Dharin Shah <[email protected]> wrote: > > What we were able to cover reliably: > - keyword progression (IN DATABASE) > - database name completion after IN DATABASE postgres > - offering SET/RESET after the database name (<TAB><TAB>) > - SET keyword completion and GUC name completion (SET work_<TAB> -> > work_mem) > > What I could not make reliable in TAP is the query-driven RESET variable > listing itself. The new completion rule only triggers at “… RESET <TAB>” > (cursor immediately after RESET), so prefix insertion tests like “RESET > wo<TAB>” won’t exercise it. That leaves list-style output (“RESET > <TAB><TAB>”), which is both highly variant across readline/libedit > implementations and not reliably matchable with the current > query_until()-based harness, leading to timeouts/flakiness even though > manual interactive testing confirms it works. > > Given that, I think keeping the existing TAP coverage for the > deterministic parts (as above) plus a short comment in 010 explaining why > the RESET variable-listing output isn’t asserted is a reasonable > compromise. If there’s a preferred pattern/harness improvement to make > query-driven list output stable, I’m happy to follow that direction. > I reached the same conclusion while experimenting with TAP coverage. The query-driven RESET variable listing only triggers at the exact … RESET <TAB> position, and does not support prefix-based matching (e.g. RESET wo<TAB>). This leaves only list-style output (RESET <TAB><TAB>), which is highly dependent on the readline/libedit implementation and does not behave deterministically under the current query_until()-based test harness. In practice this leads to timeouts or flakiness, even though the behavior works correctly in interactive psql sessions. Given this, I agree that adding partial or unreliable TAP assertions would be worse than omitting them. Keeping coverage for the deterministic parts (keyword progression, database name completion, SET behavior) and documenting why RESET variable listing is not asserted seems like the most reasonable approach for now. If, in the future, the TAP harness is extended to better support query-driven list output during completion, this would be a good area to revisit. I’d be happy to help explore that direction when such a framework exists. This patch added in commitfest https://commitfest.postgresql.org/patch/6244/ Thanks, Vasuki M C-DAC,Chennai
