On Fri, 13 Mar 2026 09:45:54 GMT, Daniel Fuchs <[email protected]> wrote:
>> The "complex assert method" is the 60-line "assertCompletion" method (not >> the immediate lambda), which is *test* code in this class, rather than any >> code-under-test. >> >> I'm just wary of negative tests with overly broad scope (e.g. test that this >> N-statement sequence of calls throws some exception *somewhere*). These make >> it hard for a reader to reason about whether the exception came from the >> expected place or not, and allows the test to pass when it shouldn't (i.e. a >> false-positive). >> >> Thanks to Daniel's comment, I realise that this case isn't quite that, >> because additional specific testing *is* being done on the resulting >> exception, so if it were thrown erroneously by something else, that test >> almost certainly wouldn't continue to pass. >> >> In light of that, I'm fine with it as-is (though an explanatory comment or >> two wouldn't go amiss - e.g. on the first example at line it's used at line >> 189 or the test method JavaDoc). >> >> As it is, if I were investigating a failure of this test I wouldn't really >> understand where to start looking. > > Either form work for me. If the current form, with the completion variable, > makes lines shorter, then maybe we should keep that ? Let's keep it as-is. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30203#discussion_r2930331067
