Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Mobile] Improve Screen recording on Android for Appium 2 #55443

Merged
merged 8 commits into from
Oct 19, 2023
Merged

[Mobile] Improve Screen recording on Android for Appium 2 #55443

Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9cb9ec5
Improve Screenrecording on Android by making sure of the following:
Oct 18, 2023
6a31e64
Rename getAndroidDeviceID function to getDeviceID since it's not plat…
Oct 18, 2023
3a3be34
Fix comment
Oct 18, 2023
ce9851f
Temp: log session
Oct 18, 2023
70de242
Use adb devices list to get the first active android emulator
Oct 18, 2023
ef7e47f
Moves getAndroidEmulatorID to its own file and replaces its usage. It…
Oct 19, 2023
cea7d05
Remove unneeded filtering
Oct 19, 2023
4cf3e8b
Move setting the androidDeviceID value for Android cases only
Oct 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 76 additions & 9 deletions packages/react-native-editor/jest_ui_setup_after_env.js
View file
Open in desktop
dcalhoun marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,60 @@ jest.setTimeout( 1000000 ); // In milliseconds.

let iOSScreenRecordingProcess;
let androidScreenRecordingProcess;
let androidDeviceID;

const isMacOSEnvironment = () => {
return process.platform === 'darwin';
};

const IOS_RECORDINGS_DIR = './ios-screen-recordings';
const ANDROID_RECORDINGS_DIR = './android-screen-recordings';
const ANDROID_EMULATOR_DIR = '/sdcard/';

const getScreenRecordingFileNameBase = ( testPath, id ) => {
const suiteName = path.basename( testPath, '.test.js' );
return `${ suiteName }.${ id }`;
};

function deleteRecordingFile( filePath ) {
if ( fs.existsSync( filePath ) ) {
try {
fs.unlinkSync( filePath );
} catch ( error ) {
// eslint-disable-next-line no-console
console.error(
`Failed to delete ${ filePath }. Error: ${ error.message }`
);
}
}
}

function getFirstAvailableAndroidEmulatorID() {
try {
const adbOutput = childProcess.execSync( 'adb devices -l' ).toString();

// Split by line and extract the device ID from the first line (excluding the header)
const lines = adbOutput
.split( '\n' )
.filter( ( line ) => line && ! line.startsWith( 'List' ) );
Copy link
Member

@dcalhoun dcalhoun Oct 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, this function fails to meet the expected outcome in certain contexts.

❯ adb devices -l
List of devices attached
RFCNB05X69K            device usb:34672640X product:r8quex model:SM_G781U1 device:r8q transport_id:10
emulator-5554          device product:sdk_gphone_arm64 model:sdk_gphone_arm64 device:emulator_arm64 transport_id:14

The above is my current environment. While the E2E test suite launches the emulator — presumably because it is explicitly set — this function returns the ID for the connected physical device.

Should we update this line to instead filter by emulator or is that not a name value that we can safely presume will be consistent?

Alternatively, is this a problem that doesn't need solving? Is it likely a CI environment will find itself a state with multiple devices? Are screen recordings even used in local environments where this is more likely to occur?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we update this line to instead filter by emulator or is that not a name value that we can safely presume will be consistent?

Oh yeah, good idea, filtering by emulator makes sense when there's a real device connected.

Alternatively, is this a problem that doesn't need solving? Is it likely a CI environment will find itself a state with multiple devices? Are screen recordings even used in local environments where this is more likely to occur?

I thought about it but we do have plans to run our tests in our own building setup, so we will be running tests "locally" when that happens and we would need to publish these screen recordings as artifacts somewhere. Similar to what we do here in Gutenberg. This is not needed now because SauceLabs handles the recordings differently but once we migrate, we will need them.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the logic in ef7e47f

I moved the function to its own file so it can be shared to other files.

I've updated it to filter the devices list for emulators only.

Now it sets the device's ID for local Android builds to avoid conflicts when different emulator/devices are connected.

if ( lines.length === 0 ) {
// eslint-disable-next-line no-console
console.error( 'No available devices found.' );
return null;
}
const firstDeviceLine = lines[ 0 ];
// Extract device ID from the beginning of the line
return firstDeviceLine.split( /\s+/ )[ 0 ];
} catch ( error ) {
// eslint-disable-next-line no-console
console.error(
'Failed to fetch the first available device ID:',
error.message
);
return null;
}
}

let allPassed = true;

// eslint-disable-next-line jest/no-jasmine-globals, no-undef
Expand All @@ -36,6 +77,7 @@ jasmine.getEnv().addReporter( {
return;
}

androidDeviceID = getFirstAvailableAndroidEmulatorID();
const fileName =
getScreenRecordingFileNameBase( testPath, id ) + '.mp4';

Expand All @@ -44,22 +86,34 @@ jasmine.getEnv().addReporter( {
fs.mkdirSync( ANDROID_RECORDINGS_DIR );
}

// Use the "mkdir -p" command to create the
// ANDROID_EMULATOR_DIR directory if it doesn't exist.
try {
childProcess.execSync(
`adb -s ${ androidDeviceID } shell "mkdir -p ${ ANDROID_EMULATOR_DIR }" 2>/dev/null`
);
} catch ( error ) {
// eslint-disable-next-line no-console
console.error( `Failed to create the directory: ${ error }` );
}

androidScreenRecordingProcess = childProcess.spawn( 'adb', [
'-s',
androidDeviceID,
'shell',
'screenrecord',
'--verbose',
'--bit-rate',
'1M',
'--size',
'720x1280',
`/sdcard/${ fileName }`,
`${ ANDROID_EMULATOR_DIR }${ fileName }`,
] );

androidScreenRecordingProcess.stderr.on( 'data', ( data ) => {
// eslint-disable-next-line no-console
console.log( `Android screen recording error => ${ data }` );
} );

return;
}

Expand All @@ -85,6 +139,7 @@ jasmine.getEnv().addReporter( {
},
specDone: ( { testPath, id, status } ) => {
allPassed = allPassed && status !== 'failed';
const isTestSkipped = status === 'pending';

if ( ! isMacOSEnvironment() ) {
return;
Expand All @@ -97,9 +152,16 @@ jasmine.getEnv().addReporter( {
// Wait for kill.
childProcess.execSync( 'sleep 1' );

const recordingFilePath = `${ ANDROID_RECORDINGS_DIR }/${ fileNameBase }.mp4`;

if ( isTestSkipped ) {
deleteRecordingFile( recordingFilePath );
return;
}

try {
childProcess.execSync(
`adb pull /sdcard/${ fileNameBase }.mp4 ${ ANDROID_RECORDINGS_DIR }`
`adb -s ${ androidDeviceID } pull ${ ANDROID_EMULATOR_DIR }${ fileNameBase }.mp4 ${ ANDROID_RECORDINGS_DIR }`
);
} catch ( error ) {
// Some (old) Android devices don't support screen recording or
Expand All @@ -113,22 +175,27 @@ jasmine.getEnv().addReporter( {
);
}

const oldPath = `${ ANDROID_RECORDINGS_DIR }/${ fileNameBase }.mp4`;
const newPath = `${ ANDROID_RECORDINGS_DIR }/${ fileNameBase }.${ status }.mp4`;

if ( fs.existsSync( oldPath ) ) {
fs.renameSync( oldPath, newPath );
if ( fs.existsSync( recordingFilePath ) ) {
fs.renameSync( recordingFilePath, newPath );
}
return;
}

iOSScreenRecordingProcess.kill( 'SIGINT' );

const oldPath = `${ IOS_RECORDINGS_DIR }/${ fileNameBase }.mp4`;
const recordingFilePath = `${ IOS_RECORDINGS_DIR }/${ fileNameBase }.mp4`;

if ( isTestSkipped ) {
deleteRecordingFile( recordingFilePath );
return;
}

const newPath = `${ IOS_RECORDINGS_DIR }/${ fileNameBase }.${ status }.mp4`;

if ( fs.existsSync( oldPath ) ) {
fs.renameSync( oldPath, newPath );
if ( fs.existsSync( recordingFilePath ) ) {
fs.renameSync( recordingFilePath, newPath );
}
},
} );
Loading