On Tue, Mar 30, 2021 at 05:06:51PM +0000, Jacob Champion wrote:
> The key there is "if there is a failure" -- false positives need to be
> debugged too. Tests I've worked with recently for the NSS work were
> succeeding for the wrong reasons. Overly generic logfile matches ("SSL
> error"), for example.Indeed, so that's a test stability issue. It looks like a good idea to make those tests more picky with the sub-errors they expect. I see most "certificate verify failed" a lot, two "sslv3 alert certificate revoked" and one "tlsv1 alert unknown ca" with 1.1.1, but it is not something that this patch has to address IMO. > modules/ssl_passphrase_callback/t/001_testfunc.pl is where I pulled > this pattern from. I see. For this case, I see no issue as the input caught is from _PG_init() so that seems better than a wait on the logs generated. > Does unilateral log truncation play any nicer with parallel test runs? > I understand not wanting to make an existing problem worse, but it > doesn't seem like the existing tests were written for general > parallelism. TAP tests running in parallel use their own isolated backend, wiht dedicated paths and ports. > Would it be acceptable to adjust the tests for live rotation using the > logging collector, rather than a full restart? It would unfortunately > mean that we have to somehow wait for the rotation to complete, since > that's asynchronous. > > (Speaking of asynchronous: how does the existing check-and-truncate > code make sure that the log entries it's looking for have been flushed > to disk? Shutting down the server guarantees it.) stderr redirection looks to be working pretty well with issues_sql_like(). > I took a stab at this in v13: "Causes each attempted connection to the > server to be logged, as well as successful completion of both client > authentication (if necessary) and authorization." (IMO any further in- > depth explanation of authn/z and user mapping probably belongs in the > auth method documentation, and this patch doesn't change any authn/z > behavior.) > > v13 also incorporates the latest SSL cert changes, so it's just a > single patch now. Tests now cover the CN and DN clientname modes. I > have not changed the log capture method yet; I'll take a look at it > next. Thanks, I am looking into that and I am digging into the code now. -- Michael
signature.asc
Description: PGP signature
