test: Separate faucet tests from local integration tests #2985
Conversation
WalkthroughA new Jest configuration for faucet tests was introduced in the Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (9)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/xrpl/jest.config.integration.js (1)
12-12: Verify the ignore pattern for faucet tests
testPathIgnorePatternsuses regexp strings to skip test paths. Your pattern'<rootDir>/test/faucet/'may not match nested test files. Consider appending.*(e.g.,'<rootDir>/test/faucet/.*') or using a regex like'/test\/faucet\//'to ensure all faucet tests are excluded from integration runs.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
packages/xrpl/jest.config.faucet.js(1 hunks)packages/xrpl/jest.config.integration.js(2 hunks)packages/xrpl/package.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/xrpl/jest.config.faucet.js (1)
packages/xrpl/jest.config.integration.js (1)
base(2-2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (5)
packages/xrpl/package.json (1)
63-64: Added dedicated faucet and composite test scripts
The newtest:faucetandtest:allscripts correctly integrate the faucet-specific Jest config and orchestrate sequential execution of all test suites, aligning with the PR’s goal to decouple external-service tests from local integration tests.packages/xrpl/jest.config.faucet.js (4)
2-2: Import shared base Jest configuration
Reusingjest.config.base.jsmaintains consistency across all test suites and reduces duplication.
5-7: Extend test roots and set longer timeout for faucet tests
Including<rootDir>/testinrootsensures Jest discovers faucet tests, and increasingtestTimeoutto 60000 ms accommodates potential delays when interacting with public faucet endpoints.
8-11: Restrict test matching to faucet directory
ThetestMatchpatterns target only.test.tsfiles undertest/faucet/, effectively isolating faucet tests from unit and integration suites.
12-12: Distinct display name for the faucet suite
Naming this suitexrpl.js-faucetwill make it easy to identify in CI logs and local outputs.
|
I think this setup would still work even if you don't move the faucet tests to their own folder. |
|
Thanks for contributing @achaljhawar! It would be great if you can update the CI setup as well so faucet test will run on PR merge to the |
|
@achaljhawar Thanks for the PR! In continuation of #2985 (comment) it would be useful to do the following:
|
|
@achaljhawar There are conflicts in |
|
@Patel-Raj11 can you check now? |
|
nice job with this! |
|
@achaljhawar Commits must be signed before we can merge the PR. Can you enable signing at your end and sign the commits and force push? |
Co-authored-by: Raj Patel <[email protected]>
Co-authored-by: Raj Patel <[email protected]>
Co-authored-by: Raj Patel <[email protected]>
|
@Patel-Raj11 I have signed the commits and force-pushed to this branch |
* refactor(tests): separate faucet tests from local integration tests * feat: add Faucet test workflow and documentation * fix: Add missing test:faucet script * fix: execute tests one time Co-authored-by: Raj Patel <[email protected]> * fix: remove docker steps from faucet test workflow * docs: update faucet tests section in CONTRIBUTING.md * fix: remove comment from contributing.md Co-authored-by: Raj Patel <[email protected]> * fix: remove test:all from root package Co-authored-by: Raj Patel <[email protected]> * Update CONTRIBUTING.md --------- Co-authored-by: Raj Patel <[email protected]>
* update HISTORY files * enable SAV amendment * add VaultCreate tx and update autofill * build(deps-dev): bump karma from 6.4.3 to 6.4.4 (#2753) * build(deps): bump @scure/bip39 from 1.5.4 to 1.6.0 (#3004) * Fix AccountRoot ledger object (#3010) * fix AccountRoot ledger object * update HISTORY.md * fix import and lint errors * request a validated ledger * build(deps-dev): bump react from 19.0.0 to 19.1.0 (#2968) * upgrade ws dependency (#2940) Co-authored-by: achowdhry-ripple <[email protected]> * build(deps-dev): bump webpack from 5.98.0 to 5.99.9 (#3015) Bumps [webpack](https://github.com/webpack/webpack) from 5.98.0 to 5.99.9. - [Release notes](https://github.com/webpack/webpack/releases) - [Commits](webpack/webpack@v5.98.0...v5.99.9) --- updated-dependencies: - dependency-name: webpack dependency-version: 5.99.9 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: achowdhry-ripple <[email protected]> Co-authored-by: Omar Khan <[email protected]> * add Vault ledger entry and base integ test * add rippled serialized type Number. update models & tests to support it * fix: add conditional check for `PermissionValue` definition (#3018) * add conditional check for PermissionValue * update HISTORY * remove release date * add unit test * ripple-binary-codec 2.4.1 patch release (#3019) * update HISTORY * update package.json files * update package-lock.json * test: Separate faucet tests from local integration tests (#2985) * refactor(tests): separate faucet tests from local integration tests * feat: add Faucet test workflow and documentation * fix: Add missing test:faucet script * fix: execute tests one time Co-authored-by: Raj Patel <[email protected]> * fix: remove docker steps from faucet test workflow * docs: update faucet tests section in CONTRIBUTING.md * fix: remove comment from contributing.md Co-authored-by: Raj Patel <[email protected]> * fix: remove test:all from root package Co-authored-by: Raj Patel <[email protected]> * Update CONTRIBUTING.md --------- Co-authored-by: Raj Patel <[email protected]> * update number test * debug * set alias Number to SerializedNumber in coreTypes * update fee in VaultCreate integ test * up the fee * up more * debug fee * update fee with comment * add vault_info RPC with integ test * remove unused var in test * use vault_id * update vaultInfo test * add owner and seq params to vault_info request * add DomainID to vault_info response * add XRPLNumber type and VaultSet tx * rename SerializedNumber to XRPLNumber * add VaultSet to integ test * refactor DEFAULT_VAULT_WITHDRAWAL_POLICY location * fix import bug * add VaultDeposit tx * add VaultDeposit to integ test * add VaultWithdraw tx * add VaultWithdraw to integ test * add VaultClawback tx * add VaultClawback to integ test * use 2 wallets for integ test * comment out VaultClawback tx in integ test * use IOU in integ test * add ClawbackAmount type and VaultClawback to integ test * add VaultDelete tx * remove isBigInt * add VaultDelete to integ test * rename to STNumber * comment out Amount in VaultClawback integ test * Revert "comment out Amount in VaultClawback integ test" This reverts commit 416c1e1. * comment out WithdrawalPolicy in VaultCreate integ test * refactor WithdrawalPolicy * cleanup * cleanup vars * update HISTORY * rename var to VAULT_DATA_MAX_BYTE_LENGTH * use STNumber class in static function * add VaultCreate to codec-fixtures * refactor unit tests * update HISTORY --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Raj Patel <[email protected]> Co-authored-by: Chenna Keshava B S <[email protected]> Co-authored-by: achowdhry-ripple <[email protected]> Co-authored-by: Achal Jhawar <[email protected]>
5 participants
High Level Overview of Change
This PR separates faucet-specific tests (which connect to external networks) from local integration tests in the xrpl.js library.
fixes #2615
Changes include:
/test/faucet/directory structure for tests that depend on external faucet servicesfundWallet.test.tsfrom integration to faucet directoryContext of Change
Our integration test suite previously combined tests that run against a local rippled server with tests that connect to public testnet/devnet networks and use faucet services. This combination caused several issues:
This refactoring creates a clear separation between local integration tests and faucet tests, allowing them to be run independently while maintaining the ability to run all tests when needed.
Type of Change
Did you update HISTORY.md?
Test Plan
Future Tasks