-
Notifications
You must be signed in to change notification settings - Fork 2
✅ e2e: test card activation flow #716
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
Conversation
🦋 Changeset detectedLatest commit: 9766625 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 |
WalkthroughThis PR refactors Maestro E2E test flows by extracting common conditional patterns into reusable subflows with ARIA support, adds accessibility labels to the card sensitivity toggle, expands panda mocks for comprehensive E2E testing, and updates deployment certificates with a new sandbox domain. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 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 introduces a comprehensive end-to-end test for the card activation process, ensuring critical user flows are thoroughly validated. It significantly upgrades the mocking infrastructure for the Panda API, allowing for more robust and realistic simulation of card operations within the test environment. Furthermore, it refactors existing Maestro test flows by extracting common interaction patterns into reusable subflows, which enhances test maintainability and reduces redundancy. An accessibility improvement has also been made to the card component. Highlights
Changelog
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces an end-to-end test for the card activation flow, which is a great addition. The changes include creating new Maestro subflows for better reusability, enhancing the server-side mocks for Panda, and adding necessary accessibility labels. The refactoring of Maestro flows is well-done and improves maintainability. I've provided a couple of suggestions to make the Panda mocks more robust by better simulating real-world API error handling, which will increase the reliability of your tests, aligning with guidelines for handling missing card records as client-side issues.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #716 +/- ##
==========================================
+ Coverage 65.67% 66.84% +1.16%
==========================================
Files 190 201 +11
Lines 6005 6343 +338
Branches 1734 1861 +127
==========================================
+ Hits 3944 4240 +296
- Misses 1881 1923 +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:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @.maestro/subflows/tapAria.yaml:
- Around line 3-9: Replace the leading "HACK" comment with a lowercase TODO
marker and a short lowercase explanation (e.g., "TODO: handle aria selector
differences between web and native"), and tighten inline brace spacing in the
two runFlow when mappings by removing spaces immediately after "{" and before
"}" so that the when maps read when: {true: "${maestro.platform != 'web'}"} and
when: {platform: web}; keep existing keys/functions (runFlow, when, commands,
tapOn, aria) intact while making only the comment and brace-spacing changes.
In @.maestro/subflows/tapWhileAria.yaml:
- Around line 10-15: The web flow's selector mismatch: inside runFlow -> repeat
the visible check uses { id: "${aria}" } but tapOn falls back to the raw string
"${tap ?? aria}", causing failures when aria is an id; change the tapOn fallback
to use an id selector object (e.g., tapOn: { id: "${aria}" }) when tap is not
provided, or add/accept a web-specific variable like tapId and use tapOn: { id:
"${tapId ?? aria}" } so the selector shape matches visible; update references in
the runFlow/repeat block (visible, tapOn, aria, tap) accordingly.
- Around line 7-9: The two Maestro repeat blocks using "repeat" with "while: {
visible: \"${aria}\" }" (and commands: [{ tapOn: \"${tap ?? aria}\" }]) need a
max-attempts cap to avoid infinite loops; update both repeat entries to include
a times parameter (e.g., times: 10) alongside the existing while condition so
the loop stops after the given attempts if "${aria}" never becomes invisible.
In `@server/test/e2e.ts`:
- Line 88: The mock for panda.getCard currently resolves undefined for missing
IDs; change the vi.fn().mockImplementation for getCard to instead reject (throw)
when the requested cardId is not found in the cards Map so it mirrors real
panda.getCard behavior (HTTP 404). Specifically, inside the mockImplementation
for getCard check cards.get(cardId) and if falsy return a rejected Promise (or
throw an Error with a clear message/status) otherwise return the resolved card;
keep the mock function name getCard and the cards Map reference so the change is
easy to locate.
In `@server/vitest.config.mts`:
- Around line 28-43: Remove the hardcoded PANDA_E2E_PRIVATE_KEY literal in
vitest.config.mts and instead read it from the environment at runtime (e.g.
process.env.PANDA_E2E_PRIVATE_KEY or import.meta.env depending on runtime),
updating the place that currently uses the multiline string to reference that
env variable; add a clear runtime check in the same module (throw or log and
exit) if PANDA_E2E_PRIVATE_KEY is missing so CI/test runs fail fast, and remove
the private key content from the repo/history (ensure you stop committing
secrets and rotate the exposed key).
Summary by CodeRabbit
Accessibility
Chores