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 -[RLMApp baseURL] always being nil #8573

Merged
merged 10 commits into from May 21, 2024
Merged

Fix -[RLMApp baseURL] always being nil #8573

merged 10 commits into from May 21, 2024

Conversation

tgoyne
Copy link
Member

@tgoyne tgoyne commented May 3, 2024

It turns out that our sync tests have been silently failing to run on CI for quite a while. Hopefully this is the only broken thing. There's a bunch of failing things.

I had to rewrite the baseUrl tests because they were not even remotely close to working.

@tgoyne tgoyne added the no-jira-ticket Skip checking the PR title for Jira reference label May 3, 2024
@tgoyne tgoyne self-assigned this May 3, 2024
@cla-bot cla-bot bot added the cla: yes label May 3, 2024
@tgoyne tgoyne force-pushed the tg/app-baseurl branch 5 times, most recently from 5106acf to 529d6a9 Compare May 4, 2024 15:30
@tgoyne
Copy link
Member Author

tgoyne commented May 4, 2024

The sync tests now pass for me locally†. Thankfully everything broken iutside of baseURL was bugs in the tests and not bugs in the library. The sync jobs are now failing on CI because it's no longer silently ignoring errors when installing BaaS. AFAICT, installing BaaS has been broken ever since they added another private dep that Xcode Cloud doesn't have permission to access in Sept 2023, and the reason the sync tests got so much more reliable when we migrated to Xcode Cloud is that they've never actually been running. The SPM tests were failing to actually invoke setup_baas.rb at all; I fixed this and then re-disabled it because we want to at least be running the non-sync tests with asan/tsan.

† Sometimes. Creating an app in BaaS fails ~5% of the time which means there's often at least one failure while running the full test suite.

@tgoyne tgoyne requested a review from nirinchev May 4, 2024 15:40
Copy link
Collaborator

@dianaafanador3 dianaafanador3 left a comment

Choose a reason for hiding this comment

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

LGTM!, added some questions

@@ -22,8 +22,10 @@ import RealmSwift
@available(iOS 13.0, macOS 10.15, tvOS 13.0, watchOS 6.0, *)
class SwiftUITests: XCTestCase {
var realm: Realm!
@MainActor
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason we are forcing this to be in the main actor (thread)

Copy link
Member Author

Choose a reason for hiding this comment

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

All of the UI test stuff is @MainActor and produces warnings about cross-actor access if used from unconfined contexts.

@@ -16,10 +16,11 @@ install_dependencies() {
brew install swiftlint
elif [[ "$CI_WORKFLOW" == "cocoapods"* ]]; then
install_ruby
elif [[ "$CI_WORKFLOW" == "objectserver"* ]] || [[ "$target" == "swiftpm"* ]]; then
elif [[ "$CI_WORKFLOW" == "sync"* ]]; then
# elif [[ "$CI_WORKFLOW" == "sync"* ]] || [[ "$CI_WORKFLOW" == "swiftpm"* ]]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we not running the sync tests in SPM?

Copy link
Member Author

Choose a reason for hiding this comment

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

We're not running the sync tests anywhere, and trying and failing to install baas now results in the non-sync spm tests not running.

@tgoyne tgoyne merged commit 83f6804 into master May 21, 2024
114 of 122 checks passed
@tgoyne tgoyne deleted the tg/app-baseurl branch May 21, 2024 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes no-jira-ticket Skip checking the PR title for Jira reference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants