Skip to content

Reduce bcrypt cost in Auth and Hasher tests for faster test execution#436

Merged
armanist merged 2 commits intosoftberg:masterfrom
armanist:435-Reduce-bcrypt-cost-in-Auth-and-Hasher-tests-for-faster-test-execution
Mar 23, 2026
Merged

Reduce bcrypt cost in Auth and Hasher tests for faster test execution#436
armanist merged 2 commits intosoftberg:masterfrom
armanist:435-Reduce-bcrypt-cost-in-Auth-and-Hasher-tests-for-faster-test-execution

Conversation

@armanist
Copy link
Copy Markdown
Member

@armanist armanist commented Mar 23, 2026

Fixes #435

Summary by CodeRabbit

  • Tests
    • Updated unit tests for authentication and hashing to use a standardized password hasher configuration (explicit cost), improving consistency and reliability.
    • Adjusted hashing-related test expectations to reflect the configured cost and rehash behavior.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c0eaf1af-c4b0-4c92-9cb9-e0a5725af6ce

📥 Commits

Reviewing files that changed from the base of the PR and between 7baac92 and 24dc065.

📒 Files selected for processing (1)
  • tests/Unit/Auth/AuthTest.php

📝 Walkthrough

Walkthrough

Tests now construct Hasher instances with bcrypt cost explicitly set to 4 via setCost(4) across auth and hasher test files; related expectations in HasherTest were adjusted to reflect the lower-cost hashing behavior.

Changes

Cohort / File(s) Summary
Auth Adapter Tests
tests/Unit/Auth/Adapters/JwtAuthAdapterTest.php, tests/Unit/Auth/Adapters/SessionAuthAdapterTest.php
Replaced default new Hasher() with (new Hasher())->setCost(4) in test setup so adapters use cost=4 during test runs.
Auth Tests
tests/Unit/Auth/AuthTest.php
Introduced a shared Hasher instance initialized with setCost(4) in setUp() and passed it into SessionAuthAdapter and JwtAuthAdapter constructions across tests.
Hasher Tests
tests/Unit/Hasher/HasherTest.php
Set hasher cost to 4 before hashing/checking and adjusted testNeedsRehash() cost progression and expectations to validate rehash logic at the lower cost.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I thumped my paws and tuned the cost,
Four beats twelve — no time is lost.
Tests now sprint with lighter weight,
A tiny tweak, a faster state. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically summarizes the main change: reducing bcrypt cost in Auth and Hasher tests for faster execution.
Linked Issues check ✅ Passed All proposed changes from issue #435 are implemented: JwtAuthAdapterTest, SessionAuthAdapterTest, AuthTest, and HasherTest all use setCost(4) as specified.
Out of Scope Changes check ✅ Passed All changes are scoped to test files and directly address bcrypt cost reduction; no unrelated modifications are present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.64%. Comparing base (efd0249) to head (24dc065).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #436   +/-   ##
=========================================
  Coverage     82.64%   82.64%           
  Complexity     2721     2721           
=========================================
  Files           241      241           
  Lines          7305     7305           
=========================================
  Hits           6037     6037           
  Misses         1268     1268           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
tests/Unit/Auth/AuthTest.php (1)

35-35: Consider centralizing the fast test hasher to remove duplication.

(new Hasher())->setCost(4) is repeated multiple times here (and similarly in sibling tests). A small helper improves maintainability if test hashing config changes again.

♻️ Possible refactor
 class AuthTest extends AuthTestCase
 {
     private JwtToken $jwt;
+
+    private function fastHasher(): Hasher
+    {
+        return (new Hasher())->setCost(4);
+    }

     public function testAuthGetAdapter(): void
     {
-        $auth = new Auth(new SessionAuthAdapter($this->authService, $this->mailer, (new Hasher())->setCost(4)));
+        $auth = new Auth(new SessionAuthAdapter($this->authService, $this->mailer, $this->fastHasher()));

         $this->assertInstanceOf(AuthenticatableInterface::class, $auth->getAdapter());

-        $auth = new Auth(new JwtAuthAdapter($this->authService, $this->mailer, (new Hasher())->setCost(4), $this->jwt));
+        $auth = new Auth(new JwtAuthAdapter($this->authService, $this->mailer, $this->fastHasher(), $this->jwt));
     }

     public function testAuthCallingValidMethod(): void
     {
-        $auth = new Auth(new JwtAuthAdapter($this->authService, $this->mailer, (new Hasher())->setCost(4), $this->jwt));
+        $auth = new Auth(new JwtAuthAdapter($this->authService, $this->mailer, $this->fastHasher(), $this->jwt));
     }

     public function testAuthCallingInvalidMethod(): void
     {
-        $auth = new Auth(new JwtAuthAdapter($this->authService, $this->mailer, (new Hasher())->setCost(4), $this->jwt));
+        $auth = new Auth(new JwtAuthAdapter($this->authService, $this->mailer, $this->fastHasher(), $this->jwt));
     }
 }

Also applies to: 39-39, 46-46, 61-61

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/Unit/Auth/AuthTest.php` at line 35, Replace repeated inline Hasher
creation in tests with a centralized helper to produce a low-cost test hasher:
add a private helper (e.g., createFastHasher or setUp-initialized $fastHasher)
and use that in places constructing SessionAuthAdapter and Auth (referencing
Auth and SessionAuthAdapter and Hasher in tests/Unit/Auth/AuthTest.php) so all
occurrences of (new Hasher())->setCost(4) are replaced by the helper call; this
keeps test setup consistent and makes future cost changes one-place updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/Unit/Auth/AuthTest.php`:
- Line 35: Replace repeated inline Hasher creation in tests with a centralized
helper to produce a low-cost test hasher: add a private helper (e.g.,
createFastHasher or setUp-initialized $fastHasher) and use that in places
constructing SessionAuthAdapter and Auth (referencing Auth and
SessionAuthAdapter and Hasher in tests/Unit/Auth/AuthTest.php) so all
occurrences of (new Hasher())->setCost(4) are replaced by the helper call; this
keeps test setup consistent and makes future cost changes one-place updates.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bacf7745-a4a7-4cef-9c4b-1f930e239295

📥 Commits

Reviewing files that changed from the base of the PR and between efd0249 and 7baac92.

📒 Files selected for processing (4)
  • tests/Unit/Auth/Adapters/JwtAuthAdapterTest.php
  • tests/Unit/Auth/Adapters/SessionAuthAdapterTest.php
  • tests/Unit/Auth/AuthTest.php
  • tests/Unit/Hasher/HasherTest.php

@armanist armanist added the help wanted Extra attention is needed label Mar 23, 2026
@armanist armanist requested a review from andrey-smaelov March 23, 2026 10:53
@armanist armanist merged commit 5ebf23c into softberg:master Mar 23, 2026
7 checks passed
@armanist armanist deleted the 435-Reduce-bcrypt-cost-in-Auth-and-Hasher-tests-for-faster-test-execution branch March 23, 2026 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

help wanted Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reduce bcrypt cost in Auth and Hasher tests for faster test execution

2 participants