Skip to content

test: fix key_lifecycle.t plan mismatch and Bignum detection#117

Merged
atoomic merged 1 commit into
cpan-authors:mainfrom
atoomic:koan.atoomic/fix-key-lifecycle-plan
Mar 19, 2026
Merged

test: fix key_lifecycle.t plan mismatch and Bignum detection#117
atoomic merged 1 commit into
cpan-authors:mainfrom
atoomic:koan.atoomic/fix-key-lifecycle-plan

Conversation

@Koan-Bot
Copy link
Copy Markdown
Contributor

What

Fix test plan mismatch in key_lifecycle.t that causes CI failures.

Why

Two bugs combined to produce Bad plan. You planned 9 tests but ran 23:

  1. $INC{'Crypt/OpenSSL/Bignum.pm'} returns false because RSA.pm doesn't load Bignum eagerly — replaced with eval { require } for reliable detection
  2. Plan was conditional (9 + ($HAS_BIGNUM ? 14 : 0)) but skip() reports skipped tests toward the total — plan must always be 23

Testing

make test passes locally (without Bignum: 9 real + 14 skipped = 23 planned).
CI will confirm both with and without Bignum installed.

🤖 Generated with Claude Code

Comment thread t/key_lifecycle.t
Crypt::OpenSSL::RSA->import_random_seed();

my $HAS_BIGNUM = $INC{'Crypt/OpenSSL/Bignum.pm'} ? 1 : 0;
my $HAS_BIGNUM = eval { require Crypt::OpenSSL::Bignum; 1 } ? 1 : 0;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Require takes a path.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

require is perfectly legal as a bareword like:

require Crypt::OpenSSL::Bignum

BUT this is not legal of course we're not doing that here.

my $module = "Crypt::OpenSSL::Bignum";
require $module; # Illegal

Comment thread t/key_lifecycle.t Outdated
my $HAS_BIGNUM = eval { require Crypt::OpenSSL::Bignum; 1 } ? 1 : 0;

plan tests => 9 + ($HAS_BIGNUM ? 14 : 0);
plan tests => 23;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a change in tests and the test count doesn't hang on $HAS_BIGNUM why did this get altered?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bot noticed that skip keeps the count of the tests even for skipped si the calculation is not necessary

@atoomic
Copy link
Copy Markdown
Collaborator

atoomic commented Mar 17, 2026

@Koan-Bot rebase look at comments above

Two bugs:
1. $INC{} check for Bignum fails because RSA.pm doesn't load it
   eagerly — use eval { require } instead for reliable detection
2. Plan was conditional (9 without Bignum) but skip() reports skipped
   tests toward the total, so plan must always be 23

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Koan-Bot Koan-Bot force-pushed the koan.atoomic/fix-key-lifecycle-plan branch from b38791f to e27838c Compare March 18, 2026 01:12
@Koan-Bot
Copy link
Copy Markdown
Contributor Author

Rebase: test: fix key_lifecycle.t plan mismatch and Bignum detection

Branch koan.atoomic/fix-key-lifecycle-plan rebased onto main and force-pushed.

Diff: 1 file changed, 1 insertion(+), 1 deletion(-)

Review feedback was analyzed and applied.

Actions

  • Resolved merge conflicts (1 round(s))
  • Rebased koan.atoomic/fix-key-lifecycle-plan onto upstream/main
  • Force-pushed koan.atoomic/fix-key-lifecycle-plan to origin
  • CI passed

CI

CI passed.


Automated by Kōan

@atoomic atoomic marked this pull request as ready for review March 18, 2026 04:35
@timlegge
Copy link
Copy Markdown
Member

@toddr are you fine with this one

@atoomic atoomic requested a review from toddr March 19, 2026 00:56
@atoomic atoomic merged commit b29b377 into cpan-authors:main Mar 19, 2026
54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants