Message-ID: <gh-pgjdbc-pgjdbc-3665-c2973761657@github.com>
From: "sehrope (@sehrope)" <noreply+sehrope@github.com>
To: "pgjdbc/pgjdbc" <noreply+pgjdbc-pgjdbc@github.com>
Date: Sun, 15 Jun 2025 12:28:25 +0000
Subject: Re: [pgjdbc/pgjdbc] PR #3665: test: add channelBinding to SslTest
In-Reply-To: <gh-pgjdbc-pgjdbc-3665@github.com>
References: <gh-pgjdbc-pgjdbc-3665@github.com>
List-Id: <gh-pgjdbc-pgjdbc.github.com>
X-GitHub-Author-Login: sehrope
X-GitHub-Comment-Id: 2973761657
X-GitHub-Comment-Type: issue_comment
X-GitHub-Issue: 3665
X-GitHub-Repo: pgjdbc/pgjdbc
X-GitHub-Type: comment
X-GitHub-Url: https://github.com/pgjdbc/pgjdbc/pull/3665#issuecomment-2973761657
Content-Type: text/plain; charset=utf-8

If you're going to refactor the channel binding options being passed into the ScramAuthenticator then do that in its own commit. Ideally in its own PR so that we see it run through the existing tests without changing anything. Definitely don't do it in a single commit labeled "add channelBinding to ssltest" as it's not test related.

Separately, that SSLTest is too complicated as it is and I don't like adding even more to it. It also makes it annoying to test one slice of functionality (e.g., channel_binding related stuff) as it's one giant test that must be run all of nothing.

If you think this is both providing better coverage and is more supportable then go for it. But I'd prefer simpler tests that cover the real world combinations that people actually use as it is easier to understand what is being tested and how the test is expected to operate.