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 <rajp@ripple.com>
Co-authored-by: Raj Patel <rajp@ripple.com>
Co-authored-by: Raj Patel <rajp@ripple.com>
|
@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 <rajp@ripple.com> * 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 <rajp@ripple.com> * fix: remove test:all from root package Co-authored-by: Raj Patel <rajp@ripple.com> * Update CONTRIBUTING.md --------- Co-authored-by: Raj Patel <rajp@ripple.com>
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