add support of mobitru screen recording feature #5307

dubovik-sergey · 2024-08-14T15:31:02Z

No description provided.

codecov · 2024-08-14T15:36:37Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.64%. Comparing base (fa31539) to head (f616f60).

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #5307   +/-   ##
=========================================
  Coverage     97.63%   97.64%           
- Complexity     6941     6967   +26     
=========================================
  Files           958      961    +3     
  Lines         20153    20224   +71     
  Branches       1323     1323           
=========================================
+ Hits          19676    19747   +71     
  Misses          368      368           
  Partials        109      109

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

valfirst

also please make sure Codecov is happy

...dus-plugin-mobitru/src/main/java/org/vividus/mobitru/recording/MobitruRecordingListener.java

valfirst · 2024-08-14T15:55:07Z

vividus-plugin-mobitru/src/main/java/org/vividus/mobitru/client/MobitruFacade.java

+    String stopDeviceScreenRecording(String deviceId) throws MobitruOperationException;
+
+    /**
+     * Stop the screen recording on the device with specified UDID.
+     *
+     * @param recordingId The ID of the recording artifact
+     * @return Binary content of the recording artifact.
+     * @throws MobitruOperationException In case of any issues during start the recording.
+     */
+    byte[] downloadDeviceScreenRecording(String recordingId) throws MobitruOperationException;


in facade it makes sense to have a single method stopDeviceScreenRecording that will return a POJO with the recording ID and its binary content

do you mean to add bute[] content field to org.vividus.mobitru.client.model.ScreenRecording and then specify it in the facade method?

I'm not sure about Mobitru REST API and its mapping to model ScreenRecording, probably there should be another POJO

but downloadDeviceScreenRecording API call just returns a byte array.
how this another POJO should look like in this case ?

just create a new object in the code from the results of 2 API calls

public record ScreenRecording(String recordingId, byte[] content) { }

do you mean that I should have 2 screen recording classes?
because, right now, I already have org.vividus.mobitru.client.model.ScreenRecording, which is using as DTO for result of stopRecording API and it contains recordingId only.

do you mean that I should have 2 screen recording classes?

yes

valfirst · 2024-08-14T16:10:25Z

...s-plugin-mobitru/src/main/java/org/vividus/mobitru/selenium/MobitruCapabilitiesAdjuster.java

@@ -46,6 +51,7 @@ public Map<String, Object> getExtraCapabilities(DesiredCapabilities desiredCapab
        {
            deviceId = mobitruFacade.takeDevice(desiredCapabilities);
            mobitruFacade.installApp(deviceId, appFileName, installApplicationOptions);
+            testContext.put(KEY, deviceId);


I guess device ID should be available in session capabilities, thus I would avoid managing this data on test data

I've checked and it's not avaialble

Web iOS - capabilities from server:
{ "appium:dc" : "US", "appium:networkConnectionEnabled" : false, "appium:takesScreenshot" : true, "appium:automationName" : "XCUITest", "appium:locationContextEnabled" : false, "appium:platformVersion" : "17.3", "appium:webDriverAgentUrl" : "http://mstf-node:7504", "appium:databaseEnabled" : false, "appium:udid" : "00008130-001958513EBA001C", "browserName" : "Safari", "appium:webStorageEnabled" : false, "appium:deviceName" : "IPHONE iPhone16,1", "appium:javascriptEnabled" : true, "platformName" : "IOS" }
I guess, it is appium:udid, isn't it?

Web Android - capabilities from server:
{ "appium:dc" : "US", "appium:chromedriverPort" : 37415, "appium:takesScreenshot" : true, "appium:warnings" : { }, "appium:desired" : { "browserName" : "Chrome", "goog:chromeOptions" : { "args" : [ ], "extensions" : [ ] }, "platformName" : "Android", "platformVersion" : "14", "deviceName" : "SAMSUNG SM-S928U1", "udid" : "127.0.0.1:37293", "dc" : "US", "chromedriverExecutableDir" : "/mount/chromedriver", "chromedriverPort" : 37415 }, "appium:deviceApiLevel" : 34, "appium:locationContextEnabled" : false, "appium:deviceScreenSize" : "1080x2340", "appium:deviceManufacturer" : "samsung", "appium:chromedriverExecutableDir" : "/mount/chromedriver", "appium:udid" : "127.0.0.1:37293", "appium:pixelRatio" : 2.8125, "goog:chromeOptions" : { "args" : [ ], "extensions" : [ ] }, "browserName" : "Chrome", "platformName" : "ANDROID", "appium:networkConnectionEnabled" : true, "appium:deviceScreenDensity" : 450, "appium:viewportRect" : { "left" : 0, "top" : 76, "width" : 1080, "height" : 2146 }, "appium:deviceModel" : "SM-S928U1", "appium:platformVersion" : "14", "appium:databaseEnabled" : false, "appium:deviceUDID" : "127.0.0.1:37293", "appium:statBarHeight" : 76, "appium:webStorageEnabled" : false, "appium:appActivity" : "com.google.android.apps.chrome.Main", "appium:deviceName" : "SAMSUNG SM-S928U1", "appium:javascriptEnabled" : true, "appium:appPackage" : "com.android.chrome" }
appium:deviceUDID looks incorrect, is it Mobitru bug?

Native mobile iOS app - capabilities from server:
{ "appium:dc" : "CIS", "appium:networkConnectionEnabled" : false, "mobitru-device-search:manufacturer" : "IPAD", "appium:takesScreenshot" : true, "appium:automationName" : "XCUITest", "appium:locationContextEnabled" : false, "appium:platformVersion" : "17.2", "appium:webDriverAgentUrl" : "http://mstf-node:7480", "mobitru-device-search:model" : "iPad", "appium:databaseEnabled" : false, "appium:udid" : "00008020-000669981A43002E", "appium:webStorageEnabled" : false, "appium:deviceName" : "IPAD iPad11,1", "appium:javascriptEnabled" : true, "platformName" : "IOS", "mobitru-device-search:type" : "tablet" }
I guess, it is appium:udid, isn't it?

Native Android app - capabilities from server:
{ "appium:dc" : "EU", "appium:chromedriverPort" : 45497, "appium:takesScreenshot" : true, "appium:warnings" : { }, "appium:desired" : { "mobitru-device-search:type" : "phone", "platformName" : "Android", "mobitru-device-search:manufacturer" : "SAMSUNG", "udid" : "127.0.0.1:42833", "automationName" : "UIAutomator2", "platformVersion" : "12", "deviceName" : "SAMSUNG SM-A525F", "dc" : "EU", "chromedriverExecutableDir" : "/mount/chromedriver", "chromedriverPort" : 45497 }, "appium:deviceApiLevel" : 31, "appium:automationName" : "UIAutomator2", "appium:locationContextEnabled" : false, "appium:deviceScreenSize" : "1080x2400", "appium:deviceManufacturer" : "samsung", "appium:chromedriverExecutableDir" : "/mount/chromedriver", "appium:udid" : "127.0.0.1:42833", "appium:pixelRatio" : 2.625, "platformName" : "ANDROID", "mobitru-device-search:type" : "phone", "appium:networkConnectionEnabled" : true, "mobitru-device-search:manufacturer" : "SAMSUNG", "appium:deviceScreenDensity" : 420, "appium:viewportRect" : { "left" : 0, "top" : 87, "width" : 1080, "height" : 2099 }, "appium:deviceModel" : "SM-A525F", "appium:platformVersion" : "12", "appium:databaseEnabled" : false, "appium:deviceUDID" : "127.0.0.1:42833", "appium:statBarHeight" : 87, "appium:webStorageEnabled" : false, "appium:deviceName" : "SAMSUNG SM-A525F", "appium:javascriptEnabled" : true }
the same as for web Android

yes and please look at p.4 where there is no real UDID of the Device:
"udid" : "127.0.0.1:42833", "appium:udid" : "127.0.0.1:42833", "appium:deviceUDID" : "127.0.0.1:42833",

only IPs

the same for Android web

so obviously it's a Mobitru bug, I believe many people are affected, is this bug going to be fixed?

this is not a bug, because this option is not required by official Appium capabilities and, according to this, this situation doesn't affect our users

it's a bug because 127.0.0.1:42833 is not UDID, if this option is not mandatory, then it could be removed, but as it's present, it contains invalid value

the bug has been reported to Support, nevertheless, there should be some class which represents Mobitru storage, e.g. MobitruTestContext, so that data sharing won't happen by re-using key and access test context

it's not a bug :) because ip:port as UDID is the normal behaviour for Android adb connection over network (not usb):

more details can be found here https://developer.android.com/tools/adb#:~:text=Confirm%20that%20your%20host%20computer%20is%20connected%20to%20the%20target%20device%3A

perform stop and download actions in the same method inside MobitruFacade

…d or driver is not initialized

valfirst · 2024-08-20T08:49:31Z

Tested in #5318

dubovik-sergey and others added 3 commits August 13, 2024 14:03

add support of mobitru screen recording feature

2905c08

cover situation when session can be shared across entire story

cd1b809

Merge branch 'vividus-framework:master' into master

6c71368

dubovik-sergey requested a review from a team as a code owner August 14, 2024 15:31

dubovik-sergey requested review from valfirst, uarlouski and draker94 August 14, 2024 15:31

valfirst requested changes Aug 14, 2024

View reviewed changes

dubovik-sergey and others added 7 commits August 19, 2024 16:08

introduce an additional DTO class for storing recording metadata

e4e098f

perform stop and download actions in the same method inside MobitruFacade

add tests for cucumber after/before scenario events

f7b2410

Merge branch 'master' into master

4238f96

Merge branch 'master' into master

b510afa

add negative cases to verify that recording will not start if disable…

ffb0516

…d or driver is not initialized

fix formatting issues

3ebf448

add test to cover situation when recording cannot be downloaded

f616f60

dubovik-sergey requested a review from valfirst August 19, 2024 15:53

valfirst added 4 commits August 20, 2024 00:43

polishing

f54343c

enable video recording in system tests

ab33d51

fix SQ violations

4ac8857

improve error handling

64c6474

valfirst merged commit e41e651 into vividus-framework:master Aug 20, 2024
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support of mobitru screen recording feature #5307

add support of mobitru screen recording feature #5307

dubovik-sergey commented Aug 14, 2024

codecov bot commented Aug 14, 2024 •

edited

Loading

valfirst left a comment

valfirst Aug 14, 2024

dubovik-sergey Aug 15, 2024

valfirst Aug 15, 2024

dubovik-sergey Aug 15, 2024

valfirst Aug 15, 2024

dubovik-sergey Aug 15, 2024

valfirst Aug 15, 2024

dubovik-sergey Aug 19, 2024

valfirst Aug 14, 2024

dubovik-sergey Aug 15, 2024

valfirst Aug 15, 2024

dubovik-sergey Aug 15, 2024 •

edited

Loading

valfirst Aug 15, 2024

dubovik-sergey Aug 15, 2024

valfirst Aug 15, 2024

valfirst Aug 15, 2024

dubovik-sergey Aug 15, 2024

valfirst commented Aug 20, 2024

add support of mobitru screen recording feature #5307

add support of mobitru screen recording feature #5307

Conversation

dubovik-sergey commented Aug 14, 2024

codecov bot commented Aug 14, 2024 • edited Loading

Codecov Report

valfirst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dubovik-sergey Aug 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valfirst commented Aug 20, 2024

codecov bot commented Aug 14, 2024 •

edited

Loading

dubovik-sergey Aug 15, 2024 •

edited

Loading