Skip to content

Fix flaky router private key PEM test#378

Open
googs1025 wants to merge 1 commit into
volcano-sh:mainfrom
googs1025:fix-router-private-key-pem-test
Open

Fix flaky router private key PEM test#378
googs1025 wants to merge 1 commit into
volcano-sh:mainfrom
googs1025:fix-router-private-key-pem-test

Conversation

@googs1025
Copy link
Copy Markdown
Member

@googs1025 googs1025 commented Jun 5, 2026

Fixes a flaky router test seen in coverage:
https://github.com/volcano-sh/agentcube/actions/runs/26986069745/job/79635856049

The parsed RSA key is equivalent, but CRT internals can differ by leading zero bytes. Use rsa.PrivateKey.Equal for the assertion.

Signed-off-by: CYJiang <googs1025@gmail.com>
Copilot AI review requested due to automatic review settings June 5, 2026 02:38
@volcano-sh-bot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign vanderchen for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates JWT private key PEM unit test to compare RSA private keys by value rather than by pointer/reference, aligning the assertion with how parsed keys are represented.

Changes:

  • Replace assert.Equal with (*rsa.PrivateKey).Equal-based assertion for private key comparison.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates a test assertion in pkg/router/jwt_test.go to compare private keys using the Equal method instead of assert.Equal. No review comments were provided, and there is no additional feedback.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.00%. Comparing base (524e55e) to head (723d732).
⚠️ Report is 119 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #378       +/-   ##
===========================================
+ Coverage   47.57%   58.00%   +10.43%     
===========================================
  Files          30       34        +4     
  Lines        2819     3181      +362     
===========================================
+ Hits         1341     1845      +504     
+ Misses       1338     1151      -187     
- Partials      140      185       +45     
Flag Coverage Δ
unittests 58.00% <ø> (+10.43%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants