refactor(test): expand e2e coverage for tab bar layout direction#4072
Open
LKuchno wants to merge 5 commits into
Open
refactor(test): expand e2e coverage for tab bar layout direction#4072LKuchno wants to merge 5 commits into
LKuchno wants to merge 5 commits into
Conversation
…nario - adding ios specific tests
LKuchno
added
platform:ios
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands the Detox E2E coverage for the “tab bar layout direction” scenario, specifically to validate iOS-only combinations where the OS writing direction and React Native I18nManager overrides are intentionally set to disagree.
Changes:
- Expanded the existing
test-tabs-tab-bar-layout-directionDetox suite with two new iOS-only scenario blocks (system LTR + RN forced RTL, and system RTL + RN forced LTR viaallowRTL: false). - Reduced redundant setup work by switching the LTR suite hook from
beforeEachtobeforeAll. - Updated default-state assertions so the direction picker reflects the resolved effective direction (
ltr/rtl) rather thaninherit, and refreshed scenario documentation to indicate complete E2E coverage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| FabricExample/e2e/single-feature-tests/tabs/test-tabs-tab-bar-layout-direction.e2e.ts | Adds iOS-only direction-mismatch coverage, adjusts setup hooks, and updates label/order assertions. |
| apps/src/tests/single-feature-tests/tabs/test-tabs-tab-bar-layout-direction/scenario.md | Updates the scenario doc to reflect that E2E coverage now spans all listed LTR/RTL configurations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 'iOS only: Tab Bar Layout Direction - system settings: LTR and RN settings: RTL', | ||
| () => { | ||
| beforeAll(async () => { | ||
| await device.launchApp({ newInstance: true }); |
Comment on lines
+260
to
+265
| it('overrides system RTL settings and renders the tab bar in ltr order', async () => { | ||
| await selectDirection('ltr'); | ||
| await expectTab1ToBeLeftOfTab2(true); | ||
| }); | ||
|
|
||
| it('follows RN RTL settings when direction is set to inherit', async () => { |
| }, | ||
| ); | ||
| describeIfiOS( | ||
| 'Tab Bar Layout Direction - system settings: RTL and RN settings: LTR', |
…ios before all runs to make sure the settings are correct
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
FabricExample/e2e/single-feature-tests/tabs/test-tabs-tab-bar-layout-direction.e2e.ts:248
- This iOS-only suite changes the RN RTL configuration (
allowRTL: false) and reloads RN, butafterAllonly relaunches with different iOS writing-direction launchArgs. BecauseI18nManager.allowRTL/forceRTLcan persist across relaunches, this risks leaking the forced-LTR RN setting into later Detox tests. Add cleanup that restores RN RTL settings to defaults (allowRTL=true, forceRTL=false) and restarts so later suites start from a known baseline.
afterAll(async () => {
await launchAppWithAttributes({
AppleTextDirection: 'NO',
NSForceRightToLeftWritingDirection: 'NO',
I18NIsRTL: 'NO',
});
});
FabricExample/e2e/single-feature-tests/tabs/test-tabs-tab-bar-layout-direction.e2e.ts:329
- This iOS-only suite sets
forceRTL: trueand reloads RN, butafterAllonly relaunches the app without resetting the persistedI18nManagerRTL configuration. That can leave the whole app in RTL for subsequent Detox test files. Add cleanup to restoreforceRTL/allowRTLdefaults and restart so the rest of the e2e run isn’t affected.
afterAll(async () => {
await launchAppWithAttributes();
});
Comment on lines
+218
to
+224
| await device.launchApp({ | ||
| newInstance: true, | ||
| launchArgs: { | ||
| AppleTextDirection: 'YES', | ||
| NSForceRightToLeftWritingDirection: 'YES', | ||
| I18NIsRTL: 'YES', | ||
| }, |
Comment on lines
+163
to
171
| await expect(element(by.id('is-rtl-information'))).toHaveText( | ||
| 'I18nManager.isRTL == true', | ||
| ); | ||
| await expect(element(by.id('react-force-rtl-picker'))).toHaveLabel( | ||
| 'forceRTL: false', | ||
| ); | ||
| await expect(element(by.id('react-allow-rtl-picker'))).toHaveLabel( | ||
| 'allowRTL: true', | ||
| ); |
Collaborator
Author
There was a problem hiding this comment.
It's as expected, we want to set RN RTL and keep it set for this test suites. afterAll function relaunch app setting direction back to LTR.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
test-tabs-tab-bar-layout-directionDetox suite to cover two previously missing iOS-only scenarios: system LTR + React Native forced RTL, and system RTL + React NativeallowRTL: false(forced LTR). These scenarios test the interaction between the OS-level writing direction and the React NativeI18nManageroverrides independently, which the previous cross-platform tests could not exercise.beforeEach→beforeAllregression in the LTR suite's setup hook, preventing redundant app reloads between tests.directionpicker now correctly initialises toltr/rtl(reflecting the resolved effective direction) rather than the previously expectedinherit.E2E testfield fromOther: Covers first two manual scenariostoYes: Covers all scenarios, and removes the stale note about iOS-specific tests being pending.Changes
describeIfiOSblocks covering (1) system LTR + RN forced RTL and (2) system RTL + RNallowRTL: false, each with default-state, explicit direction override, and cycle-through testsbeforeEachtobeforeAllin the LTR suite to avoid redundantreloadReactNativecalls between individual test casestoHaveLabelassertions for the direction picker default state ('direction: ltr'/'direction: rtl'instead of'direction: inherit')system/RN settings: LTR|RTLto clarify they cover both OS and React Native direction settingstest-tabs-tab-bar-layout-direction/scenario.mdto mark E2E coverage as complete and remove the "iOS tests pending" note