test: Shared tests for internal wallet methods#1047
Merged
Conversation
BREAKING CHANGE: HathorWallet.changeServer now returns Promise<void> instead of void, matching the service facade signature. Added changeServer to IHathorWallet. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move getServerUrl, getNetwork, getNetworkObject and getVersionData tests into shared/internal.test.ts, running against both fullnode and service facades via describe.each. Includes direct fullnode /version cross-check for both facades. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move changeServer tests into shared/server_changes.test.ts, running against both facades. The test now uses try/finally to guarantee server URL revert even on assertion failure. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move debug methods and storage reload tests into fullnode-specific/internal.test.ts. Removes the entire 'internal methods' describe block from the monolithic hathorwallet_others.test.ts file. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove unused config/loggers imports from walletservice_facade.test.ts. Apply eslint autofix for let-to-const in server_changes.test.ts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fix getVersionData cross-check to map snake_case keys from the raw fullnode API to camelCase facade keys. Split server_changes into shared (portable contract) and fullnode-specific (getVersionData validation). Add originalServerUrl to IWalletTestAdapter for safe changeServer revert across facades. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1047 +/- ##
=======================================
Coverage 87.86% 87.86%
=======================================
Files 114 114
Lines 8828 8828
Branches 2006 2006
=======================================
Hits 7757 7757
Misses 1043 1043
Partials 28 28 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
__tests__/integration/shared/internal.test.ts (2)
116-134:timestampfield is not verified against direct fullnode response.The
rawToFacademapping omitstimestamp, so it's not compared against the fullnode's raw response. Iftimestampshould also be verified for consistency, add it to the mapping.♻️ Add timestamp to mapping
const rawToFacade: Record<string, string> = { + timestamp: 'timestamp', version: 'version', network: 'network',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/integration/shared/internal.test.ts` around lines 116 - 134, The test misses verifying the timestamp field: update the rawToFacade mapping used in the loop (the rawToFacade constant) to include the timestamp mapping (map raw 'timestamp' to the facade key used on the response object, e.g., 'timestamp'), so the loop that checks fullnodeData and versionData (variables fullnodeData and versionData) will assert presence and equality for timestamp as well.
100-112: Inconsistent error handling may cause confusing test failures.If Axios throws without a response (e.g., network timeout), the catch block returns
{}, and then line 112 assertsstatus === 200on an empty object, causing a confusing failure. Consider either:
- Letting the error propagate for clearer diagnostics, or
- Skipping assertions when the request fails.
♻️ Clearer error handling
const directResponse = await axios .get('version', { baseURL: FULLNODE_URL, headers: { 'Content-Type': 'application/json' }, - }) - .catch(e => { - loggers.test!.log(`Received an error on /version: ${e}`); - if (e.response) { - return e.response; - } - return {}; }); - expect(directResponse.status).toBe(200); + // If the request fails, let it throw for clear test diagnostics + expect(directResponse.status).toBe(200);Or if you want to handle errors gracefully:
const directResponse = await axios .get('version', { baseURL: FULLNODE_URL, headers: { 'Content-Type': 'application/json' }, }) .catch(e => { loggers.test!.log(`Received an error on /version: ${e}`); - if (e.response) { - return e.response; - } - return {}; + throw new Error(`Direct fullnode request failed: ${e.message}`); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/integration/shared/internal.test.ts` around lines 100 - 112, The test's Axios call swallows network errors by returning an empty object, causing a misleading expect(directResponse.status). Update the error handling around axios.get (the directResponse variable) so that network/errors without e.response do not return `{}`; either rethrow the error after logging (so the test fails with the original error) or return/skip the assertion when no e.response is present (e.g., assert only when directResponse && directResponse.status). Ensure you reference the axios.get call, the directResponse variable, and the expect(directResponse.status) assertion when making the change.__tests__/integration/shared/server_changes.test.ts (2)
59-62: Consider a more idiomatic assertion.
resolves.not.toThrow()is unconventional for Promise assertions. SincechangeServerreturnsPromise<void>, consider:♻️ Proposed refinement
it('should accept a new server URL without throwing', async () => { const newUrl = 'https://node1.testnet.hathor.network/v1a/'; - await expect(wallet.changeServer(newUrl)).resolves.not.toThrow(); + await expect(wallet.changeServer(newUrl)).resolves.toBeUndefined(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/integration/shared/server_changes.test.ts` around lines 59 - 62, Test uses the unconventional "resolves.not.toThrow()" for a Promise<void>; update the assertion in the it block so it asserts the promise resolves cleanly—either call await wallet.changeServer(newUrl) directly or use Jest's promise matcher like expect(wallet.changeServer(newUrl)).resolves.toBeUndefined(); target wallet.changeServer in the test to make this change.
51-57: Guard against undefined wallet in afterAll cleanup.If
adapter.createWallet()throws inbeforeAll,walletremainsundefined, and theafterAllblock will crash when callingwallet.changeServer(...). Consider adding a guard or usingtry/finallywithin the test itself (similar to the pattern infullnode-specific/server_changes.test.ts).♻️ Proposed fix
afterAll(async () => { // Revert to the adapter's original server URL before stopping. // This is critical because changeServer modifies global config // that persists across tests. - await wallet.changeServer(adapter.originalServerUrl); - await adapter.stopWallet(wallet); + if (wallet) { + await wallet.changeServer(adapter.originalServerUrl); + await adapter.stopWallet(wallet); + } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/integration/shared/server_changes.test.ts` around lines 51 - 57, The afterAll cleanup assumes wallet is defined and will throw if adapter.createWallet() failed; update the afterAll to guard against undefined wallet by checking if (wallet) before calling wallet.changeServer(adapter.originalServerUrl) and adapter.stopWallet(wallet), or refactor the test to use try/finally around adapter.createWallet()/test logic so cleanup only runs when wallet was created; reference the wallet variable and the methods wallet.changeServer and adapter.stopWallet to locate and protect the cleanup calls.__tests__/integration/fullnode-specific/server_changes.test.ts (1)
55-60: Timing assertion may be flaky.Line 59 asserts
timestamp > serverChangeTime + 200, but this assumes cumulative delays are at least 200ms. Network variability could cause intermittent failures. Consider capturing a fresh baseline after the revert:♻️ More robust timing check
await delay(100); + const revertTime = Date.now().valueOf(); // Verifying the revert to the privatenet const networkData = await gWallet.getVersionData(); - expect(networkData.timestamp).toBeGreaterThan(serverChangeTime + 200); + expect(networkData.timestamp).toBeGreaterThan(revertTime); expect(networkData.network).toStrictEqual(FULLNODE_NETWORK_NAME);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/integration/fullnode-specific/server_changes.test.ts` around lines 55 - 60, The timing assertion is flaky because it relies on a fixed 200ms buffer; instead capture a fresh baseline immediately around the revert and assert the returned timestamp falls between the known serverChangeTime and that fresh baseline. Concretely, after the revert but before/just after calling gWallet.getVersionData(), record const checkTime = Date.now() and then assert networkData.timestamp > serverChangeTime && networkData.timestamp <= checkTime + 50 (or another small margin) while keeping the network equality assertion to FULLNODE_NETWORK_NAME; reference gWallet.getVersionData(), networkData.timestamp, serverChangeTime and FULLNODE_NETWORK_NAME when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@__tests__/integration/adapters/service.adapter.ts`:
- Line 53: Replace the hardcoded originalServerUrl value with a dynamic value
obtained from the same config/setup used by tests: read the base URL produced by
initializeServiceGlobalConfigs() (or the service global config it sets) and
assign that to originalServerUrl so revert/cleanup in changeServer tests uses
the actual configured base URL rather than 'http://localhost:3000/dev/'. Locate
the variable originalServerUrl in the adapter and replace the literal with the
referenced configuration accessor used by initializeServiceGlobalConfigs().
In `@__tests__/integration/fullnode-specific/server_changes.test.ts`:
- Around line 31-34: The shared singleton returned by
GenesisWalletHelper.getSingleton() is left in a stopped state by calling
gWallet.stop() in afterAll; after stopping the instance, reset the module-level
singleton so future tests recreate it (e.g., add
GenesisWalletHelper.resetSingleton() or set the internal singleton variable to
null after gWallet.stop()), or alternatively avoid calling gWallet.stop() and
only call GenesisWalletHelper.clearListeners(); ensure the chosen change updates
afterAll to either not stop the singleton or to call the new reset method so
getSingleton() returns a fresh instance for subsequent tests.
---
Nitpick comments:
In `@__tests__/integration/fullnode-specific/server_changes.test.ts`:
- Around line 55-60: The timing assertion is flaky because it relies on a fixed
200ms buffer; instead capture a fresh baseline immediately around the revert and
assert the returned timestamp falls between the known serverChangeTime and that
fresh baseline. Concretely, after the revert but before/just after calling
gWallet.getVersionData(), record const checkTime = Date.now() and then assert
networkData.timestamp > serverChangeTime && networkData.timestamp <= checkTime +
50 (or another small margin) while keeping the network equality assertion to
FULLNODE_NETWORK_NAME; reference gWallet.getVersionData(),
networkData.timestamp, serverChangeTime and FULLNODE_NETWORK_NAME when making
the change.
In `@__tests__/integration/shared/internal.test.ts`:
- Around line 116-134: The test misses verifying the timestamp field: update the
rawToFacade mapping used in the loop (the rawToFacade constant) to include the
timestamp mapping (map raw 'timestamp' to the facade key used on the response
object, e.g., 'timestamp'), so the loop that checks fullnodeData and versionData
(variables fullnodeData and versionData) will assert presence and equality for
timestamp as well.
- Around line 100-112: The test's Axios call swallows network errors by
returning an empty object, causing a misleading expect(directResponse.status).
Update the error handling around axios.get (the directResponse variable) so that
network/errors without e.response do not return `{}`; either rethrow the error
after logging (so the test fails with the original error) or return/skip the
assertion when no e.response is present (e.g., assert only when directResponse
&& directResponse.status). Ensure you reference the axios.get call, the
directResponse variable, and the expect(directResponse.status) assertion when
making the change.
In `@__tests__/integration/shared/server_changes.test.ts`:
- Around line 59-62: Test uses the unconventional "resolves.not.toThrow()" for a
Promise<void>; update the assertion in the it block so it asserts the promise
resolves cleanly—either call await wallet.changeServer(newUrl) directly or use
Jest's promise matcher like
expect(wallet.changeServer(newUrl)).resolves.toBeUndefined(); target
wallet.changeServer in the test to make this change.
- Around line 51-57: The afterAll cleanup assumes wallet is defined and will
throw if adapter.createWallet() failed; update the afterAll to guard against
undefined wallet by checking if (wallet) before calling
wallet.changeServer(adapter.originalServerUrl) and adapter.stopWallet(wallet),
or refactor the test to use try/finally around adapter.createWallet()/test logic
so cleanup only runs when wallet was created; reference the wallet variable and
the methods wallet.changeServer and adapter.stopWallet to locate and protect the
cleanup calls.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f7553bed-18c2-45d0-8ec3-bbc7848b6b82
📒 Files selected for processing (11)
__tests__/integration/adapters/fullnode.adapter.ts__tests__/integration/adapters/service.adapter.ts__tests__/integration/adapters/types.ts__tests__/integration/fullnode-specific/internal.test.ts__tests__/integration/fullnode-specific/server_changes.test.ts__tests__/integration/hathorwallet_others.test.ts__tests__/integration/shared/internal.test.ts__tests__/integration/shared/server_changes.test.ts__tests__/integration/walletservice_facade.test.tssrc/new/wallet.tssrc/wallet/types.ts
- Use config.getWalletServiceBaseUrl() getter instead of hardcoded URL for originalServerUrl in service adapter - Stop clearing genesis singleton in server_changes to avoid breaking subsequent tests - Replace flaky timing assertion with bounded check - Rethrow network errors instead of swallowing them - Use resolves.toBeUndefined() over resolves.not.toThrow - Guard afterAll cleanup against undefined wallet Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@__tests__/integration/adapters/service.adapter.ts`:
- Around line 54-56: The getter originalServerUrl currently calls
config.getWalletServiceBaseUrl() each time and can return a mutated value after
wallet.changeServer(); instead, snapshot the value once and return that stable
snapshot. Create a private field (e.g., originalServerUrlValue) initialized once
(either at class construction or module initialization) by calling
config.getWalletServiceBaseUrl(), and update the originalServerUrl getter to
return that field so cleanup uses the original, unmutated URL.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5acaa044-0aaf-4517-86b5-812197ea94d8
📒 Files selected for processing (4)
__tests__/integration/adapters/service.adapter.ts__tests__/integration/fullnode-specific/server_changes.test.ts__tests__/integration/shared/internal.test.ts__tests__/integration/shared/server_changes.test.ts
✅ Files skipped from review due to trivial changes (1)
- tests/integration/shared/internal.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/integration/fullnode-specific/server_changes.test.ts
- tests/integration/shared/server_changes.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@__tests__/integration/adapters/service.adapter.ts`:
- Around line 54-68: Remove the duplicate class declaration for
ServiceWalletTestAdapter so only one export class ServiceWalletTestAdapter
remains; consolidate the implementations by keeping the getter originalServerUrl
and the suiteSetup() implementation that initializes this._originalServerUrl
(the version at lines referencing the initialization used by the getter) and
delete the duplicate class header and its redundant suiteSetup() method,
ensuring only a single suiteSetup() sets this._originalServerUrl and calls
GenesisWalletServiceHelper.start().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 77914c40-db41-44b8-91da-c9a63db263eb
📒 Files selected for processing (1)
__tests__/integration/adapters/service.adapter.ts
Comment on lines
+54
to
+68
| export class ServiceWalletTestAdapter implements IWalletTestAdapter { | ||
| private _originalServerUrl?: string; | ||
|
|
||
| get originalServerUrl(): string { | ||
| if (!this._originalServerUrl) { | ||
| throw new Error('originalServerUrl not initialized. Call suiteSetup() first.'); | ||
| } | ||
| return this._originalServerUrl; | ||
| } | ||
|
|
||
| async suiteSetup(): Promise<void> { | ||
| initializeServiceGlobalConfigs(); | ||
| this._originalServerUrl = config.getWalletServiceBaseUrl(); | ||
| await GenesisWalletServiceHelper.start(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="__tests__/integration/adapters/service.adapter.ts"
class_count=$(rg -c '^\s*export class ServiceWalletTestAdapter\b' "$file")
suite_setup_count=$(rg -c '^\s*async suiteSetup\s*\(' "$file")
printf 'class_count=%s\nsuiteSetup_count=%s\n' "$class_count" "$suite_setup_count"
echo
rg -n '^\s*export class ServiceWalletTestAdapter\b|^\s*async suiteSetup\s*\(' "$file"Repository: HathorNetwork/hathor-wallet-lib
Length of output: 337
🏁 Script executed:
cat -n __tests__/integration/adapters/service.adapter.ts | head -120Repository: HathorNetwork/hathor-wallet-lib
Length of output: 5396
Remove the duplicate class header at line 54.
The file contains two export class ServiceWalletTestAdapter declarations (lines 45 and 54), creating a syntactically invalid nested class. Removing line 54 consolidates the class. This also requires keeping only one suiteSetup() method—retain the implementation at lines 64–68 (which properly initializes _originalServerUrl used by the getter at lines 57–62) and remove the duplicate at lines 100–103.
🧰 Tools
🪛 Biome (2.4.9)
[error] 54-54: expected a semicolon to end the class property, but found none
(parse)
[error] 54-54: expected a semicolon to end the class property, but found none
(parse)
[error] 54-54: expected a semicolon to end the class property, but found none
(parse)
[error] 54-54: expected a semicolon to end the class property, but found none
(parse)
[error] 54-54: expected a semicolon to end the class property, but found none
(parse)
[error] 54-54: Expected an identifier, a string literal, a number literal, a private field name, or a computed name but instead found '{'.
(parse)
🪛 GitHub Actions: linter
[error] 54-54: Prettier --check failed due to SyntaxError: Unexpected token. A constructor, method, accessor, or property was expected. (54:1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@__tests__/integration/adapters/service.adapter.ts` around lines 54 - 68,
Remove the duplicate class declaration for ServiceWalletTestAdapter so only one
export class ServiceWalletTestAdapter remains; consolidate the implementations
by keeping the getter originalServerUrl and the suiteSetup() implementation that
initializes this._originalServerUrl (the version at lines referencing the
initialization used by the getter) and delete the duplicate class header and its
redundant suiteSetup() method, ensuring only a single suiteSetup() sets
this._originalServerUrl and calls GenesisWalletServiceHelper.start().
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
tuliomir
force-pushed
the
test/internal-methods-shared
branch
from
49c57ab to
e0e11ac
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
__tests__/integration/adapters/service.adapter.ts (1)
63-67:⚠️ Potential issue | 🔴 CriticalCritical: Duplicate
suiteSetup()method — the second definition at lines 99-102 overrides this one.The class contains two
suiteSetup()methods. In TypeScript, the second definition (lines 99-102) overrides the first (lines 63-67), so_originalServerUrlis never initialized and the getter will always throw.Remove the duplicate at lines 99-102 to keep the correct implementation that initializes
_originalServerUrl:🐛 Proposed fix
- async suiteSetup(): Promise<void> { - initializeServiceGlobalConfigs(); - await GenesisWalletServiceHelper.start(); - }Also applies to: 99-102
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/integration/adapters/service.adapter.ts` around lines 63 - 67, The class has two suiteSetup() definitions causing the later one to override the intended initializer; remove the duplicate suiteSetup() (the second definition) so the remaining suiteSetup() calls initializeServiceGlobalConfigs(), assigns this._originalServerUrl = config.getWalletServiceBaseUrl(), and awaits GenesisWalletServiceHelper.start(); ensure the getter relying on _originalServerUrl now reads the initialized field.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@__tests__/integration/adapters/service.adapter.ts`:
- Around line 63-67: The class has two suiteSetup() definitions causing the
later one to override the intended initializer; remove the duplicate
suiteSetup() (the second definition) so the remaining suiteSetup() calls
initializeServiceGlobalConfigs(), assigns this._originalServerUrl =
config.getWalletServiceBaseUrl(), and awaits GenesisWalletServiceHelper.start();
ensure the getter relying on _originalServerUrl now reads the initialized field.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b358004e-be03-4245-9406-afd880067c80
📒 Files selected for processing (1)
__tests__/integration/adapters/service.adapter.ts
Rename 'should reload the storage' to 'should call processHistory when connection state changes to CONNECTED' to reflect what the test actually verifies. Remove clearListeners() calls from afterAll hooks in our new files. No test code registers 'new-tx' listeners on the genesis wallet — clearListeners strips the wallet's own internal handler instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add testnetServerUrl to IWalletTestAdapter so the shared test can validate changeServer via getVersionData against real testnet endpoints on both facades. Replace the weak resolves.toBeUndefined assertion with a full change→validate→revert→validate cycle that proves the server change actually took effect. Remove fullnode-specific/server_changes.test.ts which is now covered by the shared test. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
|
||
| originalServerUrl = FULLNODE_URL; | ||
|
|
||
| testnetServerUrl = 'https://node1.testnet.hathor.network/v1a/'; |
Contributor
There was a problem hiding this comment.
Why are you using the published url?
Contributor
Author
There was a problem hiding this comment.
I need to switch to another valid network to validate the connection works well and the internal methods return actual content and not stale cache. The public testnet seemed like a good candidate for that.
| }); | ||
| }); | ||
|
|
||
| // This section tests methods that have side effects impacting the whole wallet. Executing it last. |
raul-oliveira
approved these changes
Mar 30, 2026
pedroferreira1
approved these changes
Apr 6, 2026
| * @inner | ||
| * */ | ||
| changeServer(newServer: string): void { | ||
| async changeServer(newServer: string): Promise<void> { |
Member
There was a problem hiding this comment.
Is this the first step of the breaking changes planned in the RFC?
Contributor
Author
There was a problem hiding this comment.
Actually, it's an unplanned change that appeared during the shared tests building.
The changeServer() wasn't discovered during the initial research of RFC 100, but this shared test suite identified this need. It's in line with all the changes proposed by the RFC, though: unifying the behaviors of both facades as much as possible.
github-project-automation
bot
moved this from In Review (Done)
to Waiting to be deployed
in Hathor Network
raul-oliveira
added a commit
that referenced
this pull request
Apr 10, 2026
* feat: export all types and utils from public API (#1034) * Test: Shared tests for `start` (#1032) * Merge pull request #1046 from HathorNetwork/refact/fee-test-suite refact: move fee test suite to the right folder * test: add new fee template integration tests (#1048) * refact: move fee test suite to the right folder * review changes * refact: keep only moved tests, remove new template tests * test: add new fee template integration tests * fix: native token version defaulting to deposit (#1054) * test: Shared tests for internal wallet methods (#1047) BREAKING CHANGE: HathorWallet.changeServer now returns Promise<void> instead of void, matching the service facade signature. Added changeServer to IHathorWallet. * fix: precalculated wallet address derivation bug (#1056) * feat: single address policy (#1038) * feat: single address policy * chore: improve CI integration test logs (#1049) --------- Co-authored-by: André Abadesso <andre.abadesso@gmail.com> Co-authored-by: Tulio Miranda <tulio.mir@gmail.com> Co-authored-by: André Carneiro <andreluizmrcarneiro@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Acceptance Criteria
Summary
Second step of the integration test file splitting: extracts the "internal methods" block from
hathorwallet_others.test.tsand the "wallet public methods" block fromwalletservice_facade.test.tsinto dedicated, specifically-scoped test files.getServerUrl,getNetwork,getNetworkObject,getVersionData) now run against both facades viadescribe.eachchangeServercontract test verifies both facades accept URL changes without throwinggetVersionDatavalidationoriginalServerUrltoIWalletTestAdapterfor safe server revert across facades/versioncross-check to verify both facades return consistent dataBreaking change
HathorWallet.changeServer(url)signature changed fromvoidtoPromise<void>, matching the service facade.changeServerwas also added to theIHathorWalletinterface.Migration: Add
awaitto anychangeServer()calls. Callers already usingawait(like the service facade) are unaffected.New file structure
Summary by CodeRabbit
New Features
Tests