chore(e2e): detox config general cleanup and ios management #3412

KrzysztofWojnar · 2025-11-20T16:32:23Z

Description

This is first of three PRs in the scope of labs issue 569.

Realizing plans from labs discussion 572.

Changes

e2e tests could be run in more parametrized way - choosing ios version/device before the start.
CI_AVD_NAME and DETOX_AVD_NAME env variables combined into RNS_E2E_AVD_NAME
RNS_ADB_NAME environment variable introduced for local testing against attached [physical] devices.
Common e2e scripts restructured.

Screenshots / GIFs

Here you can add screenshots / GIFs documenting your change.

You can add before / after section if you're changing some behavior.

Before

Choosing apple simulator by changing config file for "ios.simulator" entry

After

Running test script with optional RNS_E2E_APPLE_SIM_NAME and RNS_E2E_IOS_VERSION environment variables.

Checklist

Included code example that can be used to test this change
Updated TS types
Updated documentation:
Ensured that CI passes

kkafar

Hey! Nice job. I have few remarks & questions. Please answer them.

According to https://github.com/software-mansion/react-native-screens-labs/discussions/572 should I expect support for E2E_ANDROID_API_LEVEL in this PR or what are plans for that?

If you want to structure the code this way, i. e. extract the "device" detecting code to separate module, then we should do analogous thing with Android related code. It can be done in separate PR, I don't mind, but let's create ticket for this, so that we do not forget.

kkafar · 2025-11-20T20:20:49Z

scripts/e2e/detox-utils.cjs

+const { iosDevice } = require('./ios-devices');

-const CI_AVD_NAME = 'e2e_emulator';
+const CI_ADB_NAME = 'e2e_emulator';


Writing from top of my head but:

ADB ~= Android Debug Bridge,
AVD ~= Android Virtual Device.

I believe this variable describes the name of the emulator (avd) we want to use -> avd makes more sense here. What do you think?

Yes, you are right
when I was doing the cleanup I was sure that we assign it to "adbName" in detox-utils, but we assign it to "avdName" in this case, I'm going to correct that

Oh, no,
This specific variable seems to be used in two places and it looks to me that one case is wrong:
We used it as a devices.attached.device.adbName in line 104->102 and it may be assigned to devices.emulator.device.avdName because of line 46->47

I suspect one of these entries was a dead code.

The name we specify is clearly an AVD name, not ADB name. Therefore let's name it appropriately, please.

I took a look into Detox docs and it seems that two fields are allowed:

avdName - for emulator name as listed by emulators -list-avd

adbName - for emulator device hash (? not sure what that hash-like id is) that can be used with ADB to identify the device.

Seems that for the wole time we've been just using the wrong field, unless the e2e CI workflow specifies e2e_emulator as the adb name of the device it creates.

PS: I've just checked and it seems that we specify avd-name field there. Not sure of it exact semantics, but it seems that our detox setup worked so far only by accident. Let's fix that.

ok, I'll replace CI_AVD_NAME and DETOX_AVD_NAME with RNS_E2E_AVD_NAME (from https://github.com/software-mansion/react-native-screens-labs/discussions/572#discussioncomment-15031761)

kkafar · 2025-11-20T20:44:40Z

scripts/e2e/ios-devices.js

@@ -0,0 +1,37 @@
+const DEFEAULT_APPLE_MODEL = 'iPhone 17';
+const DEVICE_MODEL = process.env.E2E_APPLE_DEVICE || DEFEAULT_APPLE_MODEL


DEVICE_MODEL is not really a constant here. I'd prefer it being extracted to a function and computed there.

Similarly to detectLocalAndroidEmulator function defined in the "main" util module.

For me, it makes no difference (okay, now that you've pointed it out, I'd write it camelCase).

Anyway, turned into a function

scripts/e2e/ios-devices.js

kkafar

Answered

kkafar · 2025-11-21T09:51:01Z

scripts/e2e/detox-utils.cjs

+const { iosDevice } = require('./ios-devices');

-const CI_AVD_NAME = 'e2e_emulator';
+const CI_ADB_NAME = 'e2e_emulator';


The name we specify is clearly an AVD name, not ADB name. Therefore let's name it appropriately, please.

I took a look into Detox docs and it seems that two fields are allowed:

avdName - for emulator name as listed by emulators -list-avd

adbName - for emulator device hash (? not sure what that hash-like id is) that can be used with ADB to identify the device.

Seems that for the wole time we've been just using the wrong field, unless the e2e CI workflow specifies e2e_emulator as the adb name of the device it creates.

PS: I've just checked and it seems that we specify avd-name field there. Not sure of it exact semantics, but it seems that our detox setup worked so far only by accident. Let's fix that.

Co-authored-by: Kacper Kafara <[email protected]>

…roid devices

kkafar

I still have some more remarks. I feel like we're proceeding here in a bit chaotic manner, with some unrequested changes being applied here.

I'll open a diff with suggested changes, so that we can proceed more smoothly. Will link it here once a do

kkafar · 2025-11-21T17:17:48Z

.github/workflows/android-e2e-test-fabric.yml

          force-avd-creation: false
          emulator-options: -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none
-          avd-name: e2e_emulator
+          rns_e2e_avd_name: e2e_emulator


avd-name is a configuration option from the github action this job step uses. Renaming here does not make much sense. I think you went too far with find & replace 😅

kkafar · 2025-11-21T17:53:32Z

scripts/e2e/detox-utils.cjs

+  return process.env.RNS_E2E_AVD_NAME || isRunningCI
+    ? DEFAULT_CI_AVD_NAME
+    : detectLocalAndroidEmulator();


I have few remarks to this code:

It introduces unnecessary uncertainty how to interpret this. Might be a skill issue on my end, but I prefer to have it written in such way I don't have to wonder whether || operator has higher precedence than ternary ?: operator in JavaScript or not:

(process.env.RNS_E2E_AVD_NAME || isRunningCI) ? DEFAULT_CI_AVD_NAME : detectLocalAndroidEmulator();

or

process.env.RNS_E2E_AVD_NAME || (isRunningCI ? DEFAULT_CI_AVD_NAME : detectLocalAndroidEmulator());

?

Why do we change this code at all here? From all I see we haven't started passing emulator name from CI through env, it is still hard coded name, therefore there is no need to change this code like that.

I confess, I assumed we were going to pass this value all the time, but we used hardcoded CI_AVD_NAME as a fallback due to a misconfig 😅 but that part was fine all along. That's also the reason I touched those github configs.
I'm withdrawing this now

fixed with commit 3fbdad3

... and replaced with solution from e4074bd and 8c2044b

kkafar · 2025-11-21T19:33:10Z

I've posted suggested review changes here: #3414

This is just cleaner

t0maboro · 2025-11-24T09:31:41Z

scripts/e2e/ios-devices.js

+  const passedVersion = process.env.RNS_E2E_IOS_VERSION;
+  if (passedVersion) {


I assume that the passed version should have some specific format, so I'd add some documentation and additional validation that it matches the proper regex

configuration will be passed to Detox and I planned to rely on validation in Detox itself. However, now I think you're right, because for as long as I can remember, iPhones have had consistent naming conventions and I can expect them to maintain it.

scripts/e2e/ios-devices.js

scripts/e2e/detox-utils.cjs

scripts/e2e/ios-devices.js

t0maboro · 2025-11-24T11:56:48Z

scripts/e2e/ios-devices.js

+  const envVariableKey = 'RNS_E2E_IOS_VERSION';
+  const passedVersion = process.env[envVariableKey];
+  if (passedVersion) {
+    if (/^(iOS)\s.+/.test(passedVersion)) {


nit: I haven't made up my mind, but we'll have a rather fixed number of checks; therefore, I'm wondering if startsWith won't improve readability compared to a regex. Treat it as a nitpick; for me, it's okay to leave it in its current form.

t0maboro · 2025-11-24T11:56:51Z

scripts/e2e/ios-devices.js

+  const envVariableKey = 'RNS_E2E_APPLE_SIM_NAME';
+  const passedDevice = process.env[envVariableKey];
+  if (passedDevice) {
+    if (/^(iPhone|iPad)\s.+/.test(passedDevice)) {


nit: I haven't made up my mind, but we'll have a rather fixed number of checks; therefore, I'm wondering if startsWith won't improve readability compared to a regex. Treat it as a nitpick; for me, it's okay to leave it in its current form.

kkafar self-requested a review November 20, 2025 20:12

kkafar requested changes Nov 20, 2025

View reviewed changes

KrzysztofWojnar force-pushed the @KrzysztofWojnar/e2e-config-for-detox-cleanup branch from 3797cfe to 89208a9 Compare November 20, 2025 21:42

kkafar requested changes Nov 21, 2025

View reviewed changes

KrzysztofWojnar force-pushed the @KrzysztofWojnar/e2e-config-for-detox-cleanup branch from d8762fc to d3d5c5b Compare November 21, 2025 11:07

KrzysztofWojnar changed the title ~~chore(e2e): detox config cleanup~~ chore(e2e): detox config general cleanup and ios management Nov 21, 2025

KrzysztofWojnar requested a review from kkafar November 21, 2025 12:45

KrzysztofWojnar force-pushed the @KrzysztofWojnar/e2e-config-for-detox-cleanup branch from 8192e77 to c5fa206 Compare November 21, 2025 13:13

Krzysztof Wojnar and others added 8 commits November 21, 2025 15:09

chore(e2e): detox config cleanup

8cc9c45

chore(e2e): Apply suggestions from code review (typos only)

fe083d3

Co-authored-by: Kacper Kafara <[email protected]>

chore(e2e): review comments applied

18d4c8f

chore(e2e): prefix RNS for env variables

44a73b6

chore(e2e): RNS_E2E_APPLE_DEVICE renamed to RNS_E2E_APPLE_SIM_NAME

12d4b57

chore(e2e): new env variable RNS_ADB_NAME introduced for attached and…

03d413b

…roid devices

chore(e2e): unified to RNS_E2E_AVD_NAME env var

ab2b235

chore(e2e): adbName misalignment

8fe0271

kkafar requested changes Nov 21, 2025

View reviewed changes

kkafar reviewed Nov 21, 2025

View reviewed changes

chore(e2e): avd name does NOT come from CI env variables

3fbdad3

KrzysztofWojnar force-pushed the @KrzysztofWojnar/e2e-config-for-detox-cleanup branch from c5fa206 to 3fbdad3 Compare November 21, 2025 19:31

kkafar mentioned this pull request Nov 21, 2025

review: 3412 - 1 #3414

Open

kkafar added 2 commits November 21, 2025 20:40

Add comment to DEFAULT_CI_AVD_NAME constant

eec4e70

Restore previous way of resolving avd name

3f5ee0d

This is just cleaner

KrzysztofWojnar requested a review from t0maboro November 24, 2025 09:06

t0maboro reviewed Nov 24, 2025

View reviewed changes

scripts/e2e/detox-utils.cjs Show resolved Hide resolved

chore(e2e): review suggestions

9000905

t0maboro reviewed Nov 24, 2025

View reviewed changes

chore(e2e): typos

77e2519

t0maboro approved these changes Nov 24, 2025

View reviewed changes

		@@ -0,0 +1,37 @@
		const DEFEAULT_APPLE_MODEL = 'iPhone 17';
		const DEVICE_MODEL = process.env.E2E_APPLE_DEVICE \|\| DEFEAULT_APPLE_MODEL

		const passedVersion = process.env.RNS_E2E_IOS_VERSION;
		if (passedVersion) {

Uh oh!

chore(e2e): detox config general cleanup and ios management #3412

Are you sure you want to change the base?

chore(e2e): detox config general cleanup and ios management #3412

Conversation

KrzysztofWojnar commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Screenshots / GIFs

Before

After

Checklist

Uh oh!

kkafar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KrzysztofWojnar Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KrzysztofWojnar Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kkafar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kkafar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kkafar Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KrzysztofWojnar Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kkafar commented Nov 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KrzysztofWojnar Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

t0maboro Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

t0maboro Nov 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

KrzysztofWojnar commented Nov 20, 2025 •

edited

Loading

KrzysztofWojnar Nov 20, 2025 •

edited

Loading

KrzysztofWojnar Nov 20, 2025 •

edited

Loading

kkafar Nov 21, 2025 •

edited

Loading

KrzysztofWojnar Nov 21, 2025 •

edited

Loading

KrzysztofWojnar Nov 24, 2025 •

edited

Loading

t0maboro Nov 24, 2025 •

edited

Loading

t0maboro Nov 24, 2025 •

edited

Loading