search

parse-community / Parse-Swift

The Swift SDK for Parse Platform (iOS, macOS, watchOS, tvOS, Linux, Android, Windows)
https://parseplatform.org
MIT License
308 stars 69 forks source link

fix: Adding callback queue on .success #448

Open lsmilek1 opened 12 months ago

lsmilek1 commented 12 months ago

New Pull Request Checklist

Issue Description

Parse.User, Parse.Installation, Parse.Object save method is not dispatching the result back to callback queue if the result is success. It is dispatching back to callback queue only if it fails.

Closes: https://github.com/parse-community/Parse-Swift/issues/431

Approach

Adding callback queue on .success result also.

TODOs before merging

parse-github-assistant[bot] commented 12 months ago

Thanks for opening this pull request!

codecov[bot] commented 12 months ago

Codecov Report

Attention: 19 lines in your changes are missing coverage. Please review.

Comparison is base (3d4bb13) 90.37% compared to head (9ea59d4) 90.55%.

Files Patch % Lines
Sources/ParseSwift/Objects/ParseUser.swift 86.56% 9 Missing :warning:
Sources/ParseSwift/Objects/ParseObject.swift 86.95% 6 Missing :warning:
Sources/ParseSwift/Objects/ParseInstallation.swift 93.75% 3 Missing :warning:
Sources/ParseSwift/Coding/ParseEncoder.swift 66.66% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #448 +/- ## ========================================== + Coverage 90.37% 90.55% +0.18% ========================================== Files 161 161 Lines 16267 16401 +134 ========================================== + Hits 14701 14852 +151 + Misses 1566 1549 -17 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mtrezza commented 12 months ago

@lsmilek1 You asked for me to take a look; it seems that CI tests are failing.

lsmilek1 commented 12 months ago

@mtrezza I see in the log error that builds fail:

Error: Process completed with exit code 65.

As I am new to CI (talking to mechanical engineer here) I cannot find out what I missed just by adding that few lines. In desktop xcode the build succeed. Is there any "how to" I could study to find out what where it hangs? Not even ChatGPT made me expert on this. :-)

Looking for example on the xcode-build-watchos I cannot see anything I could work with.

lsmilek1 commented 12 months ago

After trying to understand this a third evening I am confident to say I will not be able to resolve this. I don't think I should change any CI workflow configuration and cannot read anything useful from the logs here. I rolled back to 4.14.0

lsmilek1 commented 9 months ago

@mtrezza , I figured out the failing tests. Thanks to AI to explain me how all this works. :-) Would you be able to merge the changes? Thank you!

mtrezza commented 9 months ago

I believe to merge this we'd have to find out why some CI jobs are failing first.

lsmilek1 commented 9 months ago

@mtrezza, I tried in my local xcode and the test are every time successful. I noticed that jobs that succeed here (example xcode-test-ios) have the problematic test that calls backgroundQueue also failing:

✗ testThreadSafeSaveAllAsync, Asynchronous wait failed: Exceeded timeout of 20 seconds, with unfulfilled expectations: "Save object1", "Save object2".
✗ testThreadSafeSaveAllAsync, Asynchronous wait failed: Exceeded timeout of 20 seconds, with unfulfilled expectations: "Save object1", "Save object2".
✗ testThreadSafeSaveAllAsync, Asynchronous wait failed: Exceeded timeout of 20 seconds, with unfulfilled expectations: "Save object1", "Save object2".
✗ testThreadSafeSaveAllAsync, Asynchronous wait failed: Exceeded timeout of 20 seconds, with unfulfilled expectations: "Save object1", "Save object2".
✓ testThreadSafeSaveAllAsync (1.747 seconds)

Just that in these case the workflow file specifies to re-try up to 10x, what is not the case for spm-test and `spm-test5_2". As I am not experienced in this field and @cbaker6 is probably not around anymore, I am not sure if adding re-try to those spm-test is just fine or I should rather make timeout on the test expectation something very long:

wait(for: [expectation1, expectation2], timeout: 20.0) --> wait(for: [expectation1, expectation2], timeout: 100.0)

Is it ok that I adjust workflow file to re-try?

The failing codeCoverage is probably also something I will not get succeeding, yet it is required to pass

lsmilek1 commented 9 months ago

So, apparently the timeout increase helped to solve the problem. Might it be that these job test do any concurrency / background test much slower for some reason?