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

Record video using simctl #441

Merged
merged 1 commit into from
Oct 9, 2020
Merged

Record video using simctl #441

merged 1 commit into from
Oct 9, 2020

Conversation

RainNapper
Copy link
Contributor

Use the XCTest listener protocol to add hooks for xcrun simctl io [sim-id] recordVideo.

abhisood
abhisood previously approved these changes May 8, 2020
View reviewed changes
Copy link

@abhisood abhisood left a comment

Choose a reason for hiding this comment

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

Lgtm!

bp/src/BPTestBundleConnection.h Outdated Show resolved Hide resolved
bp/src/BPTestBundleConnection.m Outdated Show resolved Hide resolved
bp/src/BPTestBundleConnection.m Outdated Show resolved Hide resolved
bp/src/BPUtils.m Show resolved Hide resolved
ob
ob reviewed May 8, 2020
View reviewed changes
Copy link
Member

@ob ob left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for contributing this feature.

Could we add a test case that actually records video for a test and checks that video was recorded? I think it could be wired in one of the tests that runs BPSampleApp.

README.md Outdated Show resolved Hide resolved
bp/src/BPConfiguration.m Outdated Show resolved Hide resolved
bp/src/BPTestBundleConnection.m Show resolved Hide resolved
@RainNapper
Copy link
Contributor Author

Nice! Thanks for contributing this feature.

Could we add a test case that actually records video for a test and checks that video was recorded? I think it could be wired in one of the tests that runs BPSampleApp.

Added a test in the last push.

@RainNapper RainNapper force-pushed the rainnapper/bluepill/video branch 10 times, most recently from 5d5b5b3 to d43a4bc Compare May 18, 2020 23:19
@RainNapper RainNapper force-pushed the rainnapper/bluepill/video branch 3 times, most recently from e48eaeb to bc4baa8 Compare May 19, 2020 23:05
@RainNapper RainNapper force-pushed the rainnapper/bluepill/video branch 7 times, most recently from 1a0eb3c to f1bc12d Compare June 3, 2020 22:38
@kinwahlai
Copy link

Hi there, you may want to allow the user to change the codec option, h264 or hevc.
The default is hevc but slack cannot play hevc video file.

Suggestion:
new option deleteOnTestPassed , meaning delete video if the test passed.
It can reduce the number of Videos stored on the CI machine.

Cool feature by the way.

@RainNapper RainNapper force-pushed the rainnapper/bluepill/video branch 2 times, most recently from da72c0b to c33de3c Compare October 7, 2020 20:05
@RainNapper
Copy link
Contributor Author

Hi there, you may want to allow the user to change the codec option, h264 or hevc.
The default is hevc but slack cannot play hevc video file.

Suggestion:
new option deleteOnTestPassed , meaning delete video if the test passed.
It can reduce the number of Videos stored on the CI machine.

Cool feature by the way.

Totally agree. bp is pretty heavy on the configs atm, mind adding this as a follow up PR?

@ravimandala ravimandala self-requested a review October 9, 2020 07:56
@ravimandala
Copy link
Contributor

Hi there, you may want to allow the user to change the codec option, h264 or hevc.
The default is hevc but slack cannot play hevc video file.

Suggestion:
new option deleteOnTestPassed , meaning delete video if the test passed.
It can reduce the number of Videos stored on the CI machine.

Cool feature by the way.

@RainNapper I like @kinwahlai's suggestion. The deleteOnTestPassed can either be a config, or the default behavior too. It would be great if you can create an issue to discuss and follow it up with a PR. Thanks for the contribution.

ravimandala
ravimandala approved these changes Oct 9, 2020
View reviewed changes
Copy link
Contributor

@ravimandala ravimandala left a comment

Choose a reason for hiding this comment

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

LGTM.

@ravimandala ravimandala merged commit 26a3761 into MobileNativeFoundation:master Oct 9, 2020
ravimandala pushed a commit to ravimandala/bluepill that referenced this pull request Oct 9, 2020
@RainNapper @ravimandala
Record video using simctl (MobileNativeFoundation#441)
efe1693 
Use the XCTest listener protocol to add hooks for `xcrun simctl io [sim-id] recordVideo`.
AhmedEid added a commit to tinyspeck/bluepill that referenced this pull request Nov 18, 2020
@AhmedEid
Merge branch 'master' into DYLD_FALLBACK_LIBRARY_PATH
65fec64 
* master:
  Reference libXCTestBundleInject.dylib only if it exists (MobileNativeFoundation#460)
  Record video using simctl (MobileNativeFoundation#441)
  Xcode 12.0 support (MobileNativeFoundation#458)
  Retry app crash tests and consider then non-fatal if they pass (MobileNativeFoundation#456)
  Do not retry crashed tests (MobileNativeFoundation#443)
  Fixing minor issues and doing some minor refactoring/clean-up (MobileNativeFoundation#444)
  Xcode 11.5 support (MobileNativeFoundation#447)
  Xcode 11.4 support (MobileNativeFoundation#446)
  Add a flag to disable Xcode version check failure (MobileNativeFoundation#436)
  Support sidecar applications (MobileNativeFoundation#438)
  Added a few more tests to mock test failure scenarios and fixed a few final Exit Status issues  (MobileNativeFoundation#430)
  make retry checks more resilient to prevent infinite retries (MobileNativeFoundation#432)
  Xcode 11.3 support (MobileNativeFoundation#426)
  Supporting tests and app hosts in subfolders (MobileNativeFoundation#419)

# Conflicts:
#	bp/src/SimulatorHelper.m
#	bp/tests/SimulatorHelperTests.m
@RainNapper RainNapper deleted the rainnapper/bluepill/video branch July 6, 2021 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ob ob ob left review comments

@ravimandala ravimandala ravimandala approved these changes

@abhisood abhisood abhisood left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@RainNapper @kinwahlai @ravimandala @ob @abhisood