Skip to content

Fix sleep between retries of startRecordingScreen - #603

Merged
Nathan (hydraxman) merged 1 commit into
microsoft:mainfrom
DavidAsulin1:fix_video_sleep
Sep 27, 2023
Merged

Nathan (hydraxman) merged 1 commit into
microsoft:mainfrom
DavidAsulin1:fix_video_sleep

Conversation

@DavidAsulin1

Copy link
Copy Markdown
Contributor

Description

A minor fix for the sleep time between startRecordingScreen. The retryAndSleepWhenFalse takes sleepSeconds param and it's set to 1000 second (probably a mistake). I changed the value to 10 second (align with the other usages of this method).

Pull Request Checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Code compiles correctly with all tests are passed.
  • I've read the contributing guide and followed the recommended practices.
  • Wikis or README have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

  • Yes
  • No

How you tested it

I encounter an issue running test on a mac machine. If appium failed to start the recording the retryAndSleepWhenFalse enter a very long sleep of 1000 seconds which block the test. Changing the value to 10 second (align with the other usages of this method) did fix the issue.

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Technical design
  • Build related changes
  • Refactoring (no functional changes, no api changes)
  • Code style update (formatting, renaming) or Documentation content changes
  • Other (please describe):

@hydraxman

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
No pipelines are associated with this pull request.

@hydraxman

Nathan (hydraxman) commented Sep 26, 2023 •

Copy link
Copy Markdown
Member

David Asulin (@DavidAsulin1) thank you for the contribution!

you may need to agree to microsoft-github-policy by commenting:

@microsoft-github-policy-service agree

cc Le Zhou (@zhou9584)

@hydraxman

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
No pipelines are associated with this pull request.

@DavidAsulin1

Copy link
Copy Markdown
Contributor Author

@microsoft-github-policy-service agree

@hydraxman

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
No pipelines are associated with this pull request.

@hydraxman
Nathan (hydraxman) enabled auto-merge (squash) September 27, 2023 01:18
@hydraxman

Copy link
Copy Markdown
Member

Do we know why this keeps hanging in this state? Le Zhou (@zhou9584) OliverShen (@olivershen-wow) AichiB7A (@TedaLIEz)

image

@hydraxman
Nathan (hydraxman) merged commit 4d24a70 into microsoft:main Sep 27, 2023
AichiB7A (TedaLIEz) added a commit that referenced this pull request Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants