test(e2e): Adds Feedback Widget Maestro E2E tests #4604
Conversation
Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com>
antonis
changed the base branch from
feedback-ui-2
to
antonis/capture-screenshot-error
…-e2e # Conflicts: # CHANGELOG.md # packages/core/android/src/oldarch/java/io/sentry/react/RNSentryModule.java # packages/core/src/js/feedback/FeedbackWidget.tsx # packages/core/src/js/feedback/ScreenshotButton.tsx # packages/core/src/js/tracing/reactnativeprofiler.tsx # packages/core/test/feedback/ScreenshotButton.test.tsx # packages/core/test/feedback/__snapshots__/FeedbackWidgetManager.test.tsx.snap
|
|
||
| - runFlow: | ||
| when: | ||
| visible: 'Engine: Hermes for RN 0.77.1' # Skipping for 0.65.3 |
There was a problem hiding this comment.
As discussed and documented the Modal implementation is fully supported on React Native 0.71 and up.
To avoid complicating the e2e-tests/cli.mjs implementation to independently run each .yml test in order to separate the feedback.yml execution I skipped the test with Maestro. We can probably iterate by injecting the <FeedbackWidget> form in the test App if we want to cover 0.65.3 with the E2E tests.
There was a problem hiding this comment.
Thank you for the note. The Modal is fully supported in 0.71 in the new architecture,
but for the legacy architecture, the test should pass (the modal should work) even for 0.65.3.
If it doesn't it might be unrelated to the modal, but we can discuss it.
There was a problem hiding this comment.
but for the legacy architecture, the test should pass (the modal should work) even for 0.65.3.
I haven't been able to successfully run the tests on 0.65.3 legacy architecture either. From the logs it seems that the modal does not open but I'll need to investigate this further to understand why this is happening.
antonis
requested review from
krystofwoldrich and
lucas-zimerman
as code owners
|
Note that |
| /> | ||
| )} | ||
| </View> | ||
|
|
||
| {config.showName && ( | ||
| <> | ||
| <Text style={styles.label}> | ||
| <Text style={styles.label} testID='name-label' accessibilityLabel={text.nameLabel}> |
There was a problem hiding this comment.
Do we need the additional testId and accessibilityLabel?
I would prefer not using the testIDs if not needed to avoid conflicting with user apps.
If we need them then lets prefix them with something unique like sentry-feedback-*.
There was a problem hiding this comment.
Updated with 8176d4f:
- I've cleaned up all the unneeded test-id and used the
sentry-feedback-*prefix for those used. - Removed the unneeded accessibility labels.
📢 Type of change
Based on: #4726
PR Chain:
📜 Description
Adds Feedback Widget Maestro E2E tests for the happy path flow for RN
0.77.1:💡 Motivation and Context
Part of #4302
💚 How did you test it?
CI (submitted E2E test feedback), Manual
Note: To run Android locally you might need to change the build architecture android.patch
📝 Checklist
sendDefaultPIIis enabled🔮 Next steps
#skip-changelog