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

fix: Adding callback queue on .success #448

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

lsmilek1
Copy link
Contributor

@lsmilek1 lsmilek1 commented Nov 26, 2023

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: #431

Approach

Adding callback queue on .success result also.

TODOs before merging

  • none, the tests and documentation stays the same

Adding callback queue
Adding callback queue
Adding callback queue
adding callback queue to all methods
adding callback queue on all methods
Copy link

parse-github-assistant bot commented Nov 26, 2023

Thanks for opening this pull request!

  • 🎉 We are excited about your hands-on contribution!

@lsmilek1 lsmilek1 changed the title Adding callback queue on .success fix: Adding callback queue on .success Nov 26, 2023
Copy link

codecov bot commented Nov 26, 2023

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 ⚠️
Sources/ParseSwift/Objects/ParseObject.swift 86.95% 6 Missing ⚠️
Sources/ParseSwift/Objects/ParseInstallation.swift 93.75% 3 Missing ⚠️
Sources/ParseSwift/Coding/ParseEncoder.swift 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mtrezza
Copy link
Member

mtrezza commented Nov 27, 2023

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

@lsmilek1
Copy link
Contributor Author

lsmilek1 commented Nov 27, 2023

@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
Copy link
Contributor Author

lsmilek1 commented Nov 28, 2023

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

adding callback queue on all methods
adding callback queue on all methods
Bump oldest xcode version to xcode_13.1
@lsmilek1
Copy link
Contributor Author

@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!

Bump oldest xcode version to xcode_13.1
Bump oldest xcode version to xcode_13.1
@mtrezza
Copy link
Member

mtrezza commented Feb 15, 2024

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

@lsmilek1
Copy link
Contributor Author

@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
Copy link
Contributor Author

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?

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.

Parse.User save not dispatching back to callback queue when result is success
2 participants