MarathonLabs / marathon

Cross-platform test runner
https://docs.marathonlabs.io
GNU General Public License v2.0
584 stars 122 forks source link

Local worker shared temporary directory not properly cleaned/synced #920

Open alvar-bolt opened 7 months ago

alvar-bolt commented 7 months ago

Describe the bug After updating from Marathon 0.8.4 to 0.10.0, we ran into an interesting bug where we are unable to run tests of one app after another specific app has been tested by given runner. The reported issue from Xcode is:

Failed to get launch progress for <XCUIApplicationImpl: 0x600000ce5b90 com.redacted.secondApp at /private/tmp/marathon/shared/appUnderTest.app>: Simulator device returned an error for the requested operation. Failed to create promise. (Underlying Error: Failed to set app extension placeholders for com.redacted.secondApp. Failed to create promise. (Underlying Error: Attempted to set app extension placeholder promise with bundle ID com.redacted.firstApp.someWidget that does not match required prefix of com.redacted.secondApp. for parent. Mismatched bundle IDs.))

To Reproduce Steps to reproduce the behaviour:

  1. Have 2 apps. One with app extension and second without it.
  2. Configure Marathondevices to have a local worker:
    workers:
    - transport:
      type: local
    devices:
      - type: simulatorProfile
        deviceType: com.apple.CoreSimulator.SimDeviceType.iPhone-11
  3. Run marathon for first app (with extension)
  4. Run marathon for second app (without extension)
  5. Observe that second app test fails

Expected behavior Second app is able to run successfully.

Logs and reports Marathon version: 0.10.0

After second app failure, when looking into /private/tmp/marathon/shared/appUnderTest.app/ directory, we can see that there is Plugins directory with first app extension .appex bundle. This should not be there and is the source of the failed test run.

Devices (please complete the following information):

Additional context This bug does not happen on ssh workers. After looking around the source code, I believe that the difference comes from how these directories are pushed. For ssh, Rsync is used and it has delete option enabled: https://github.com/MarathonLabs/marathon/blob/07a5883eaf4725dc65214a0735bdce98307d43a2/vendor/vendor-apple/base/src/main/kotlin/com/malinskiy/marathon/apple/cmd/remote/rsync/RsyncFileBridge.kt#L161 For local runs, as I understand, JvmFileBridge is used and it does not remove "extra" files that are present in destination but not in source.

One solution that worked for me locally was to add removing of "remoteSharedDirectory" in device preparing phase here: https://github.com/MarathonLabs/marathon/blob/07a5883eaf4725dc65214a0735bdce98307d43a2/vendor/vendor-apple/ios/src/main/kotlin/com/malinskiy/marathon/apple/ios/AppleSimulatorDevice.kt#L186 But I'm not sure if this so correct solution and will it work with ssh workers or will if break sharing of artefacts between simulators.

If maintainers could provide bit information on how ssh worker and simulators artefact preparation steps work exactly, I would gladly submit PR to fix this bug.

(I believe this issue started after new "shared artefacts" functionality was introduced: https://github.com/MarathonLabs/marathon/pull/840/files)

Malinskiy commented 7 months ago

Hey @alvar-bolt . Thanks for submitting such a detailed issue. I'm pretty sure that removing the remote directory will impact performance of incremental transfer using rsync for ssh workers. A better solution would be to fix this in the JvmFileBridge itself. There are multiple ways to make sure the same behavior happens. One can be just deleting the target directory on push. Another could be using rsync in the local file bridge since rsync works locally as well as remotely.

I'd be happy to review a PR with a potential fix. Let me know if you need any more pointers.