refactor: fix spelling issues in tests #6199

mvadari · 2026-01-09T16:43:09Z

High Level Overview of Change

This PR removes the src/tests exception from the cspell config and fixes all the issues that arise as a result. No functionality/test change.

Context of Change

More cspell work, follow-up from #5719

Type of Change

Refactor (non-breaking change that only restructures code)

API Impact

N/A

Test Plan

CI passes.

Copilot

Pull request overview

This PR enables spell checking for test files by removing the src/test/** exception from the cspell configuration and systematically fixes all spelling issues found in test code. The changes include improved variable naming conventions, better function names, grammar fixes in comments, and more realistic test data strings.

Key Changes:

Updated cspell configuration to enable spell checking on test files with appropriate string literal ignores
Renamed functions to use proper camelCase (e.g., testCTIDRPC → testCtidRPC, createjv → createJV)
Improved variable names for clarity and removed possessives (e.g., bobsOffer → bobOffer, enableds → enabledAmendments)

Reviewed changes

Copilot reviewed 36 out of 36 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
.config/cspell.config.yaml	Removed test directory from ignore list, added test file string literal ignores, added CTID regex pattern, and new dictionary words
src/test/rpc/Transaction_test.cpp	Renamed `testCTIDRPC` to `testCtidRPC` for proper camelCase
src/test/rpc/NoRipple_test.cpp	Renamed `gline0` to `gwLine0` for clarity
src/test/rpc/LedgerEntry_test.cpp	Renamed `alicesAcctRootBinary` to `aliceAcctRootBinary` (removed possessive)
src/test/rpc/Handler_test.cpp	Added cspell comment to ignore "stdev"
src/test/rpc/AccountObjects_test.cpp	Renamed `bobs_account_objects` to `bob_account_objects` (removed possessive)
src/test/rpc/AccountLines_test.cpp	Fixed comment grammar from "beckys" to "becky's" and "alices" to "alice's"
src/test/protocol/STAmount_test.cpp	Renamed `smallXsmall` to `smallXSmall` for proper camelCase
src/test/overlay/compression_test.cpp	Improved variable names from `jrequestUsd`/`jreply_usd` to `requestUsd`/`replyUSD`
src/test/ledger/View_test.cpp	Renamed `enableds` to `enabledAmendments` for clarity
src/test/ledger/Directory_test.cpp	Fixed comment grammar from "Alices creates" to "Alice creates"
src/test/jtx/mpt.h	Renamed functions to proper camelCase: `createjv` → `createJV`, `destroyjv` → `destroyJV`, etc.
src/test/jtx/impl/mpt.cpp	Updated function implementations and error messages to match header changes
src/test/jtx/impl/TestHelpers.cpp	Renamed `allpe` to `allPathElements` for clarity
src/test/jtx/impl/AMM.cpp	Renamed `jvflags` to `jvFlags` and `jvres` to `jvRes` for proper camelCase
src/test/jtx/TestHelpers.h	Updated function declaration to match implementation
src/test/core/Config_test.cpp	Updated test data strings to use hyphens in domain names for realism
src/test/app/XChain_test.cpp	Added cspell comments to ignore "NonBatch" in function name and "attns" abbreviation
src/test/app/TrustAndBalance_test.cpp	Renamed `invoiceid` to `invoiceId` for proper camelCase
src/test/app/SHAMapStore_test.cpp	Renamed `oinfo` to `outInfo` for clarity
src/test/app/ReducedOffer_test.cpp	Removed possessives from variable names: `bobsFee` → `bobFee`, `bobsOffer` → `bobOffer`, etc.
src/test/app/PayStrand_test.cpp	Renamed `xrpsi` to `xrpStepInfo` and updated function call from `allpe` to `allPathElements`
src/test/app/Offer_test.cpp	Removed possessives from variable names throughout
src/test/app/NFToken_test.cpp	Renamed `nftOnlyXRPID` to `nftOnlyXrpID` and similar variables for consistency
src/test/app/MPToken_test.cpp	Fixed comment grammar from "bobs'" to "bob's"
src/test/app/Loan_test.cpp	Added cspell comment to ignore "LOANTODO"
src/test/app/LoanBroker_test.cpp	Fixed spelling in comment from "AllowTrustLineClaback" to "AllowTrustLineClawback"
src/test/app/LPTokenTransfer_test.cpp	Fixed comment grammar from "bobs's" to "bob's"
src/test/app/FixNFTokenPageLinks_test.cpp	Fixed comment grammar from "alices's" to "alice's"
src/test/app/EscrowToken_test.cpp	Fixed comment grammar and renamed `testIOUINSF` to `testIOUInsufficientFunds`
src/test/app/DepositAuth_test.cpp	Renamed `readedCreds` to `readCreds` for proper past tense
src/test/app/CrossingLimits_test.cpp	Renamed `bobsOfferCount` to `bobOfferCount` and `evitasOfferCount` to `evitaOfferCount`
src/test/app/Batch_test.cpp	Fixed comment grammar from "bobs" to "bob's"
src/test/app/AMM_test.cpp	Fixed spelling of "nataly" to "natalie" and improved comment grammar
src/test/app/AMMExtended_test.cpp	Renamed `AMMXRPPool` to `AmmXrpPool` and updated function call
src/test/app/AMMCalc_test.cpp	Renamed type `trates` to `transfer_rates`, variables for clarity, and fixed comment

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

codecov · 2026-01-09T17:24:25Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.4%. Comparing base (778da95) to head (dd7d547).

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #6199     +/-   ##
=========================================
- Coverage     79.4%   79.4%   -0.0%     
=========================================
  Files          839     839             
  Lines        71619   71619             
  Branches      8238    8235      -3     
=========================================
- Hits         56855   56850      -5     
- Misses       14764   14769      +5

see 3 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

Copilot reviewed 38 out of 38 changed files in this pull request and generated no new comments.

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

bthomee · 2026-01-09T21:36:00Z

include/xrpl/proto/xrpl.proto

-  tsCOMMITED = 3;  // in a closed ledger
+  tsNEW = 1;        // origin node did/could not validate
+  tsCURRENT = 2;    // scheduled to go in this ledger
+  tsCOMMITTED = 3;  // in a closed ledger


This will be a breaking change for those using the compiled proto. However, the number on the wire stays the same, which is a positive. It would be an easy fix for anyone using this proto whose code will fail to compile, but it would be a surprise if not properly communicated.

A way to more gracefully fix this:

Add option allow_alias = true; to the enum definition.

Add [deprecated = true]; and the cspell ignore to the tsCOMMITED entry.

Add tsCOMMITTED = 3; below the deprecated value.

Then in 6 months we can remove the deprecated value and remove the alias stanza, essentially achieving what you have right now.

Why is this xrpl.proto change a problem but all the ones in #5719 not?

It's the same problem there. I missed that they were proto enums and not C++ enums. If @godexsoft is ok with the proto changes and it is unlikely to severely affect downstream consumers (maybe it's only Clio?) then we can go ahead. Otherwise it would be wise to add aliases to all modified enum values.

Clio only uses the org/xrpl/v1/*.proto so i don't think this would break anything for us. I'm not aware of any other consumers of proto and their use cases though.

src/test/app/AMM_test.cpp

src/test/app/XChain_test.cpp

bthomee · 2026-01-09T21:48:54Z

cspell.config.yaml

 suggestWords:
  - xprl->xrpl
-  - unsynched->unsynced
+  - unsynched->unsynced # cspell: disable-line not sure what this problem is....


CSpell checks its own config file and trips over it, is that what happens here?

I guess? I was confused as to why only this one was flagged though

Co-authored-by: Bart <bthomee@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 54 out of 54 changed files in this pull request and generated no new comments.

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

Copilot

Pull request overview

Copilot reviewed 38 out of 38 changed files in this pull request and generated no new comments.

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

Copilot

Pull request overview

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

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

Copilot · 2026-01-12T18:29:23Z

include/xrpl/proto/xrpl.proto

-  tsCOMMITED = 3;  // in a closed ledger
+  tsNEW = 1;        // origin node did/could not validate
+  tsCURRENT = 2;    // scheduled to go in this ledger
+  tsCOMMITTED = 3;  // in a closed ledger


The spelling correction from "tsCOMMITED" to "tsCOMMITTED" changes the enum value name in a protobuf file. This is a breaking API change that will affect any code that uses this enum value. The enum value name is part of the external API and changing it will break compatibility with existing code that references "tsCOMMITED".

Copilot · 2026-01-12T18:29:24Z

src/test/overlay/compression_test.cpp

+            requestUsd[jss::tx_json] =
                pay("bob", "alice", bob["USD"](fund / 2));
-            Json::Value jreply_usd = wsc->invoke("sign", jrequestUsd);
+            Json::Value replyUSD = wsc->invoke("sign", requestUsd);


There is an inconsistent naming convention here. The variable on line 201 is named "requestUsd" (with lowercase 'd'), while the variable on line 205 is named "replyUSD" (with uppercase 'SD'). For consistency, both should follow the same casing convention - either both use "Usd" or both use "USD". Given that the first was renamed from "jrequestUsd" to "requestUsd", it would be more consistent to name the second variable "replyUsd" rather than "replyUSD".

@mvadari given that I've seen USD/XRP in all caps used a lot in variable names, renaming requestUsd to requestUSD seems to be the most consistent in my opinion. It would then also match the casing used by replyUSD.

I was going for the smallest diff, but made this change
dd7d547

Copilot

Pull request overview

Copilot reviewed 39 out of 39 changed files in this pull request and generated no new comments.

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

bthomee

Some minor comments.

bthomee · 2026-01-22T18:56:15Z

src/test/jtx/impl/mpt.cpp

        // Outstanding amount is reduced by the transfer fee if any
-        env_.require(mptbalance(*this, issuer_, outstnAmt - (actual - amount)));
+        env_.require(
+            mptbalance(*this, issuer_, outstandingAmt - (actual - amount)));


I'm wondering whether the mptbalance class should be renamed to MptBalance or MPTBalance.

All the JTX classes are lowercase

bthomee · 2026-01-22T18:57:31Z

src/test/overlay/compression_test.cpp

+            requestUsd[jss::tx_json] =
                pay("bob", "alice", bob["USD"](fund / 2));
-            Json::Value jreply_usd = wsc->invoke("sign", jrequestUsd);
+            Json::Value replyUSD = wsc->invoke("sign", requestUsd);


@mvadari given that I've seen USD/XRP in all caps used a lot in variable names, renaming requestUsd to requestUSD seems to be the most consistent in my opinion. It would then also match the casing used by replyUSD.

Copilot

Pull request overview

Copilot reviewed 39 out of 39 changed files in this pull request and generated no new comments.

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

refactor: fix spelling issues in tests

1599966

Copilot AI review requested due to automatic review settings January 9, 2026 16:43

Copilot started reviewing on behalf of mvadari January 9, 2026 16:43 View session

Copilot AI reviewed Jan 9, 2026

View reviewed changes

more fixes

1747b9f

fix regexes

a353fe5

Copilot AI review requested due to automatic review settings January 9, 2026 17:27

mvadari requested a review from a team as a code owner January 9, 2026 17:27

Copilot started reviewing on behalf of mvadari January 9, 2026 17:28 View session

mvadari requested a review from bthomee January 9, 2026 17:28

Copilot AI reviewed Jan 9, 2026

View reviewed changes

move files into .config

cba9ab0

bthomee reviewed Jan 9, 2026

View reviewed changes

Update src/test/app/AMM_test.cpp

ee08bd2

Co-authored-by: Bart <bthomee@users.noreply.github.com>

Copilot AI review requested due to automatic review settings January 9, 2026 22:22

Copilot started reviewing on behalf of mvadari January 9, 2026 22:23 View session

Copilot AI reviewed Jan 9, 2026

View reviewed changes

Merge branch 'develop' into cspell-tests

e2efd5d

mvadari force-pushed the cspell-tests branch from 4612552 to e2efd5d Compare January 9, 2026 22:26

fix one more typo

d22d56b

Copilot AI review requested due to automatic review settings January 9, 2026 22:51

Copilot started reviewing on behalf of mvadari January 9, 2026 22:52 View session

Copilot AI reviewed Jan 9, 2026

View reviewed changes

mvadari and others added 2 commits January 12, 2026 12:03

Merge branch 'develop' into cspell-tests

ebd0252

Merge branch 'develop' into cspell-tests

ff0dab1

Copilot AI review requested due to automatic review settings January 12, 2026 18:25

Copilot started reviewing on behalf of mvadari January 12, 2026 18:25 View session

Copilot AI reviewed Jan 12, 2026

View reviewed changes

Merge branch 'develop' into cspell-tests

005b21a

fix typo

0612230

Copilot AI review requested due to automatic review settings January 22, 2026 18:14

Copilot started reviewing on behalf of mvadari January 22, 2026 18:14 View session

Copilot AI reviewed Jan 22, 2026

View reviewed changes

bthomee approved these changes Jan 22, 2026

View reviewed changes

mvadari and others added 2 commits January 26, 2026 09:54

Merge branch 'develop' into cspell-tests

77b7f67

requestUsd -> requestUSD

dd7d547

Copilot AI review requested due to automatic review settings January 26, 2026 14:57

Copilot started reviewing on behalf of mvadari January 26, 2026 14:57 View session

Copilot AI reviewed Jan 26, 2026

View reviewed changes

refactor: fix spelling issues in tests #6199

Are you sure you want to change the base?

refactor: fix spelling issues in tests #6199

Conversation

mvadari commented Jan 9, 2026

High Level Overview of Change

Context of Change

Type of Change

API Impact

Test Plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

codecov bot commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

bthomee left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

codecov bot commented Jan 9, 2026 •

edited

Loading