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.

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

ci: update OS versions in mobile test matrix & fix flaky tests #1035

Merged
merged 24 commits into from
Nov 1, 2022

Conversation

vaind
Copy link
Collaborator

@vaind vaind commented Oct 21, 2022

      # This slows down the CI significantly so let's see if
      - name: Record test session
        shell: bash
        run: |
          set -x
          command -v ffmpeg || brew install ffmpeg
          mkdir -p ${{ env.ARTIFACTS_PATH }}
          ffmpeg -codecs
          ffmpeg -nostdin -f avfoundation -i 0 -video_size 720x480 -framerate 5 -an ${{ env.ARTIFACTS_PATH }}out.mkv > ${{ env.ARTIFACTS_PATH }}ffmpeg.log 2>&1 &

... steps to run tests

      - name: Stop screen recording
        continue-on-error: true
        run: killall ffmpeg

@vaind vaind force-pushed the ci/reenable-flaky-tests branch 6 times, most recently from 3d5ad93 to 3fc6ca4 Compare October 25, 2022 10:26
@vaind vaind changed the title ci: update OS versions in mobile test matrix ci: update OS versions in mobile test matrix & fix flaky tests Oct 25, 2022
@vaind vaind force-pushed the ci/reenable-flaky-tests branch 7 times, most recently from 3e6f292 to 78715c5 Compare October 28, 2022 13:00
@vaind vaind marked this pull request as ready for review October 28, 2022 21:02
.github/workflows/ci.yml Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
uses: reactivecircus/android-emulator-runner@76c2bf6f95ed6458fd659a1bdb680a0f8df232dc
id: smoke-test
timeout-minutes: 30
continue-on-error: true
Copy link
Contributor

Choose a reason for hiding this comment

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

We will see it in the overview but it does not break CI?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is how it was before a recent PR - I've removed this line there and tried to implement a different logic. Didn't work. Putting it back.

disk-size: 4096M # Some runs have out of storage error when installing the smoke test.
emulator-options: -no-snapshot-save -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none
disable-animations: false
emulator-options: -no-snapshot-save -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none -accel on
Copy link
Contributor

Choose a reason for hiding this comment

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

-no-window is no longer necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not sure it ever was.

.github/workflows/ci.yml Show resolved Hide resolved
Comment on lines +40 to +44
if (Test-Path env:CI)
{
# Take Screenshot of VM to verify emulator start
screencapture "$(ArtifactsPath)/host-screenshot.jpg"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Was that not working because of the no-window?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

didn't even try without, as it means just having to wait for half an 75 minutes (at least) to see anything :D

scripts/smoke-test-droid.ps1 Outdated Show resolved Hide resolved
@@ -37,6 +38,7 @@
class RequestVerifier:
__requests = []
__testNumber = 0
__lock = Lock()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the lock?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's using a multithreaded server (see end of script) and there was a data race causing the collected numbers to be off

lock = Lock()


def registerUpload(name: str, chunks: int):
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to change our "expected" count of debug symbols with a boolean to check if anything got uploaded at all? To get rid of those random changes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wouldn't. I know it's not very convenient to update right now, but at least you know it's doesn't break completely (like uploading just some small bit, single chunk instead of 6, for example) by some change we do or a dependency update.

@vaind vaind merged commit 601e76a into main Nov 1, 2022
@vaind vaind deleted the ci/reenable-flaky-tests branch November 1, 2022 08:57
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.

Unity 2021 + Android API 30 Smoke test crash
3 participants