Sponsored Content
Skip to content

fix: Update clang-tidy to include src/tests directory header check - #7307

Merged
bthomee merged 4 commits into
XRPLF:developfrom
andrzej-neti:develop-neti-update-clang-fix-config
May 26, 2026
Merged

fix: Update clang-tidy to include src/tests directory header check#7307
bthomee merged 4 commits into
XRPLF:developfrom
andrzej-neti:develop-neti-update-clang-fix-config

Conversation

@andrzej-neti

Copy link
Copy Markdown
Collaborator

High Level Overview of Change

Update configuration of the clang-tidy tool to include headers from the src/tests and apply the project style to affected files.

Context of Change

The goal is to improve automated checks for the project.

API Impact

No API changes

@andrzej-neti
andrzej-neti force-pushed the develop-neti-update-clang-fix-config branch from 726e432 to b404dae Compare May 21, 2026 15:08
@mathbunnyru

Copy link
Copy Markdown
Contributor

Please, update the PR title, these are the choices: https://github.com/XRPLF/actions/blob/main/.github/workflows/check-pr-title.yml#L24

Comment thread src/tests/libxrpl/helpers/TestFamily.h
Comment thread src/tests/libxrpl/tx/AccountSet.cpp Outdated
@codecov

codecov Bot commented May 21, 2026

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #7307     +/-   ##
=========================================
- Coverage     82.4%   82.3%   -0.0%     
=========================================
  Files         1011    1011             
  Lines        76330   76330             
  Branches      7318    7316      -2     
=========================================
- Hits         62874   62850     -24     
- Misses       13456   13480     +24     

see 12 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@andrzej-neti andrzej-neti changed the title config: update clang-tidy to include src/tests directory header check fix: update clang-tidy to include src/tests directory header check May 23, 2026
@andrzej-neti
andrzej-neti force-pushed the develop-neti-update-clang-fix-config branch from a5ab8fe to 8ed3708 Compare May 23, 2026 20:49
@marek-foss-neti marek-foss-neti changed the title fix: update clang-tidy to include src/tests directory header check fix: Update clang-tidy to include src/tests directory header check May 26, 2026
@andrzej-neti
andrzej-neti force-pushed the develop-neti-update-clang-fix-config branch from 8ed3708 to 1525e2a Compare May 26, 2026 12:41
@mathbunnyru
mathbunnyru requested a review from Copilot May 26, 2026 13:41

Copilot AI left a comment

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.

Pull request overview

This PR updates the repository’s clang-tidy configuration so that headers under src/tests are included in the header-filtered checks, and then applies style/clang-tidy-driven cleanups to the affected test helper code to keep the build and checks passing.

Changes:

  • Expand .clang-tidy HeaderFilterRegex to include tests (in addition to test) directories.
  • Apply clang-tidy/style fixes in src/tests/libxrpl (designated initializers, nested namespace syntax, naming alignment like Account::kMaster, and adding [[nodiscard]] on getters/overrides).
  • Add a local suppression for the intentionally-uppercase XRP helper function name.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/tests/libxrpl/tx/AccountSet.cpp Uses designated initializers for test case data to satisfy readability/style checks.
src/tests/libxrpl/helpers/TxTest.h Adds a targeted NOLINT for the XRP helper name under clang-tidy naming rules.
src/tests/libxrpl/helpers/TxTest.cpp Updates genesis/master account usage to the renamed Account::kMaster.
src/tests/libxrpl/helpers/TestServiceRegistry.h Applies style updates (nested namespace, member naming) aligned with clang-tidy.
src/tests/libxrpl/helpers/TestFamily.h Minor style adjustments (nested namespace, [[nodiscard]], pointer usage in reset()).
src/tests/libxrpl/helpers/IOU.h Formatting/style tweaks in constructor initializer list.
src/tests/libxrpl/helpers/Account.h Renames master to kMaster and adds [[nodiscard]] to accessors.
src/tests/libxrpl/helpers/Account.cpp Renames the static definition to Account::kMaster.
.clang-tidy Updates header filter regex to include headers under tests directories.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +52 to 53
IOU(Currency currency, Account const& issuer) : currency_(currency), issuer_(issuer.id())
{
Comment thread src/tests/libxrpl/helpers/TestFamily.h
@mathbunnyru
mathbunnyru requested a review from kuznetsss May 26, 2026 14:08

@kuznetsss kuznetsss left a comment

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.

LGTM

@mathbunnyru mathbunnyru added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label May 26, 2026
@bthomee
bthomee added this pull request to the merge queue May 26, 2026
Merged via the queue into XRPLF:develop with commit 85af406 May 26, 2026
3 checks passed
@bthomee bthomee added this to the 3.2.0 milestone May 27, 2026
Kassaking7 pushed a commit to Kassaking7/rippled that referenced this pull request Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants