✅ e2e misc improvements #702
Conversation
cruzdanilo
requested review from
dieguezguille,
franm91 and
nfmelendez
as code owners
🦋 Changeset detectedLatest commit: 9b97664 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Caution Review failedThe pull request is closed. WalkthroughAccessibility label added to the loan amount selector; Maestro flows refactored with conditional runFlow entries and tag syntax changes; Onesignal domain-to-ID mapping expanded; e2e script changed to watch mode; e2e utilities extended with concurrent wallet_sendCalls and wallet_getCallsStatus plus TX id utilities. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 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 |
Summary of ChangesHello @cruzdanilo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the end-to-end (e2e) testing suite and improves application accessibility. It introduces an Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several miscellaneous improvements for the end-to-end tests, including an accessibility improvement by adding an aria-label to the loan amount selector. My review focuses on improving code clarity and correctness. I've suggested making an error message more descriptive and refactoring a complex calculation for better readability in the E2E utility file. I also noted a missing translation key for the new aria-label which should be added to the English translation file.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #702 +/- ##
==========================================
- Coverage 64.54% 63.80% -0.74%
==========================================
Files 173 173
Lines 5378 5385 +7
Branches 1509 1512 +3
==========================================
- Hits 3471 3436 -35
- Misses 1727 1769 +42
Partials 180 180
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/loans/AmountSelector.tsx (1)
24-95: 🛠️ Refactor suggestion | 🟠 MajorAvoid the single‑letter
tabbreviation.Use a descriptive name (e.g.,
translate) for readability and to align with naming conventions.🔧 Suggested rename
- const { t } = useTranslation(); + const { t: translate } = useTranslation(); ... - aria-label={t("Amount")} + aria-label={translate("Amount")}As per coding guidelines, do not use abbreviations or cryptic names; write out full words like error, parameters, request instead of err, params, req.
🤖 Fix all issues with AI agents
In @.maestro/flows/local.yaml:
- Around line 26-28: The inline mapping/sequence in the runFlow block has extra
spaces inside braces/arrays; update the when and commands values to remove
spaces immediately after "{" and before "}" so the mapping uses {true:
"${output.portfolio === output.portfolioBefore}"} and the array items use
[{tapOn: Home},{runFlow: ../subflows/readPortfolio.yaml}] (i.e., adjust the
runFlow block's when and commands entries to eliminate inner-brace/array spacing
around the inline mappings).
- Around line 36-47: The YAML has lint issues in the two runFlow blocks: add two
spaces before the inline comment (change " # HACK..." to " # HACK...") and
remove extra spaces inside the inline mapping braces in the when clauses and
repeat while/mapping entries (e.g. change when: { true: "${maestro.platform !=
'web'}" } to when: {true: "${maestro.platform != 'web'}"} and when: { platform:
web } to when: {platform: web}, and similarly remove spaces in repeat/commands
mappings for tapOn: Home and tapOn: { id: Home } so they match the suggested
compact inline-brace style).
In `@server/script/e2e.ts`:
- Line 14: The spawn call that creates `v` in e2e.ts currently spreads
`process.env` and unconditionally enables `--watch`; instead, read required env
values once at startup via a small initializer (e.g., a getEnv/config helper)
and inject them into the spawn env object (do not spread process.env), and gate
the `--watch` arg behind an interactive flag (e.g., VITEST_WATCH or CI check) so
in CI you pass `run` (or omit `--watch`); update the `spawn("vitest", ...)`
invocation and any code that references `v` to use the new env object and flag
so the script no longer directly accesses process.env and only watches in
interactive sessions.
In `@src/utils/e2e.ts`:
- Around line 63-76: The wallet_getCallsStatus handler currently uses
Promise.all over publicClient.getTransactionReceipt which throws
TransactionReceiptNotFoundError for pending receipts and rejects the whole call;
change this to use Promise.allSettled or wrap each
publicClient.getTransactionReceipt call in try/catch (inside the Array.from
mapper) to convert not-found/pending errors into a placeholder receipt
indicating pending, then compute receipts and set status to 100 if any receipt
is pending (otherwise 200/500 as before); ensure you keep existing symbols
(params validation, sliceHex, hexToNumber, publicClient.getTransactionReceipt,
receipts array) and preserve the returned shape (version, id, atomic, receipts,
status, chainId).
6fb339d to
6d55474
Compare
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.