Message-ID: From: "vlsi (@vlsi)" To: "pgjdbc/pgjdbc" Date: Mon, 16 Jun 2025 13:05:32 +0000 Subject: Re: [pgjdbc/pgjdbc] PR #3665: test: add channelBinding to SslTest In-Reply-To: References: List-Id: X-GitHub-Author-Login: vlsi X-GitHub-Comment-Id: 2976583086 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-2976583086 Content-Type: text/plain; charset=utf-8 >run all of nothing. It takes about 20-50 seconds on my machine. I find it acceptable. What do you expect by "test one slice of functionality"? If you mean "test all cases when `channelBinding=require`", then you could do that by adding `if (channelBinding != ChannelBinding.REQUIRE) { continue; }` into `data()` method. An alternative option is to execute a single case. > SSLTest is too complicated I'm all ears on suggestions to have a comparable test coverage with "less complicated" code. https://jqwik.net/ might help with automatic shrinking, however, we don't use jqwik for testing yet. I do not find the code hard to understand, as all it does it it setups the connection properties, then it connects. If the connection succeeds it verifies there were not expected errors, and if the connection fails, it verifies if the failure was expected or not. The options do affect one another, so we should better test the way they interact rather than be sorry of not testing a thing. For instance, `ChannelBindingRequiredTest` misses a case with `client certificate` authentication. It misses a case with `sslMode=allow` (==upgrade the connection to TLS as required by `channelBinding`), and it probably misses the other important cases. Does it make sense testing "channelBinding=require + use direct ALPN connection"? Why not. `SslTest` covers it.