Skip to content

feat: add web integration test setup #3295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

venilinvasilev
Copy link
Contributor

Description:
WIP

@lfdt-bot
Copy link
Contributor

lfdt-bot commented Aug 14, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

Signed-off-by: venilinvasilev <venilin.vasilev@gmail.com>
@venilinvasilev venilinvasilev force-pushed the feat/add-web-integration-test-setup branch from bed4c6a to 694e455 Compare August 14, 2025 15:32
Signed-off-by: venilinvasilev <venilin.vasilev@gmail.com>
@venilinvasilev venilinvasilev force-pushed the feat/add-web-integration-test-setup branch from 694e455 to 981f2c0 Compare August 15, 2025 05:19
Copy link

codecov bot commented Aug 15, 2025

Codecov Report

❌ Patch coverage is 13.29787% with 163 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/LocalProviderWeb.js 0.00% 146 Missing ⚠️
src/client/WebClient.js 34.78% 15 Missing ⚠️
src/browser.js 0.00% 1 Missing ⚠️
src/constants/ClientConstants.js 93.33% 1 Missing ⚠️
Files with missing lines Coverage Δ
src/client/NodeClient.js 91.43% <100.00%> (-0.13%) ⬇️
src/browser.js 0.00% <0.00%> (ø)
src/constants/ClientConstants.js 94.11% <93.33%> (-0.06%) ⬇️
src/client/WebClient.js 43.50% <34.78%> (-0.55%) ⬇️
src/LocalProviderWeb.js 0.00% <0.00%> (ø)

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: venilinvasilev <venilin.vasilev@gmail.com>
Signed-off-by: venilinvasilev <venilin.vasilev@gmail.com>
Signed-off-by: venilinvasilev <venilin.vasilev@gmail.com>
@ivaylonikolov7
Copy link
Contributor

ivaylonikolov7 commented Aug 15, 2025

Great work so far. I’d like to propose adding two distinct React environments to test whether all transactions run successfully in both setups. This is important because the majority of our web clients use React with the SDK.

The two environments should differ in their module bundlers—one using Webpack and the other using Vite. We’ve encountered compatibility issues in the past with different bundlers, so this approach will help us proactively address potential problems.

This setup would also allow us to:

  • Integrate WalletConnect with the SDK and verify proper behavior.

  • Test functions like executeWithSigner and getReceiptWithSigner, aligning closely with real-world use cases our SDK users encounter.

  • Mitigate future problems when we return tree-shaking to the SDK.

I understand this would require considerable effort, might involve adding new development dependencies to the SDK, changes in the tests and some rework on the tests. However, the long-term benefit of reducing recurring compatibility issues makes this a worthwhile investment.

The current implementation is solid; however, I’d like to align it as closely as possible with the real-world use cases our users encounter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants