On Mon, 11 May 2026 11:54:02 GMT, Mikhail Yankelevich
<[email protected]> wrote:
>> Brent Christian has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains seven additional
>> commits since the last revision:
>>
>> - Merge branch 'master' into smartcardio
>> - update example code in javax.smartcardio package doc to use try-finally
>> - minor comment update
>> - add missed clean() calls, update comments
>> - fix imports and copyright years
>> - add test case
>> - convert CardImpl finalizer to use Cleaner
>
> test/jdk/sun/security/smartcardio/TestCleaner.java line 71:
>
>> 69: Card card = terminal.connect("*");
>> 70: if (card == null) {
>> 71: System.out.println("Skipping the test: " +
>
> This should be a skipped exception imo, it will look like it's passed
> otherwise
This test is meant only to test a specific thing - that the new cleaning action
does not prevent the Card object (CardImpl, to be precise) from being collected.
If no Card object can be obtained, this can't be tested. I think it can be
skipped. It's common for other tests in the smartcardio/ test directory to be
skipped if conditions aren't right.
In my view, if the terminal senses that a card is present, but connect() does
not yield a Card object, that indicates a problem in some other part of the
system. I would expect it to be caught by one of the other tests.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30683#discussion_r3223710938
