Skip to content

consensus/bor: add test coverage#2046

Merged
cffls merged 4 commits intodevelopfrom
consensus_bor_test
Feb 10, 2026
Merged

consensus/bor: add test coverage#2046
cffls merged 4 commits intodevelopfrom
consensus_bor_test

Conversation

@cffls
Copy link
Copy Markdown
Contributor

@cffls cffls commented Feb 9, 2026

Add unit tests for consensus/bor

@claude
Copy link
Copy Markdown

claude bot commented Feb 9, 2026

Code Review

Issue Found

File: consensus/bor/bor_test.go

Bug: Undefined function newAPIForTest

The function newAPIForTest(t) is called in these test functions but is never defined anywhere in the codebase:

  • TestAPI_GetRootHash_Valid (line 3895)
  • TestAPI_GetRootHash_Cached (line 3904)
  • TestAPI_GetRootHash_StartGreaterThanEnd (line 3918)
  • TestAPI_GetRootHash_EndBeyondHead (line 3926)
  • TestAPI_GetRootHash_MaxCheckpointExceeded (line 3934)

This will cause a compilation error:

undefined: newAPIForTest

Suggested fix: Add the newAPIForTest helper function that creates and returns a test API instance. Similar helper functions like newChainAndBorForTest (defined at line 119) exist in this file and can serve as a template.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 10, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 50.18%. Comparing base (f5ee550) to head (d5b3233).
⚠️ Report is 3 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2046      +/-   ##
===========================================
+ Coverage    49.57%   50.18%   +0.61%     
===========================================
  Files          873      871       -2     
  Lines       150595   150614      +19     
===========================================
+ Hits         74662    75591     +929     
+ Misses       70896    69984     -912     
- Partials      5037     5039       +2

see 24 files with indirect coverage changes
see 24 files with indirect coverage changes

🚀 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.

lucca30
lucca30 approved these changes Feb 10, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@lucca30 lucca30 left a comment

Choose a reason for hiding this comment

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

lgtm.
If possible, I would suggest to ask claude to look into the introduced changes in bor_test.go and check the duplicates. In general it do a pretty good job addressing this.

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Feb 10, 2026

Quality Gate Failed Quality Gate failed

Failed conditions
9.6% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@cffls
Copy link
Copy Markdown
Contributor Author

cffls commented Feb 10, 2026

lgtm. If possible, I would suggest to ask claude to look into the introduced changes in bor_test.go and check the duplicates. In general it do a pretty good job addressing this.

Thanks! I refactored another time. Duplication seems ok now.

@cffls cffls merged commit 96df644 into develop Feb 10, 2026
17 of 19 checks passed
@cffls cffls deleted the consensus_bor_test branch February 10, 2026 20:50
kamuikatsurgi pushed a commit that referenced this pull request Feb 13, 2026
@cffls @kamuikatsurgi
consensus/bor: add test coverage (#2046)
35fcb6f 
* consensus/bor: add test coverage

* Add missing files

* Refactor

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

Reviewers

@lucca30 lucca30 lucca30 approved these changes

@marcello33 marcello33 marcello33 approved these changes

@kamuikatsurgi kamuikatsurgi kamuikatsurgi approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@cffls @lucca30 @marcello33 @kamuikatsurgi