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

test(storage, ios): enable tests after upstream bug fixes released #7361

Merged
merged 4 commits into from
Sep 20, 2023

Conversation

mikehardy
Copy link
Collaborator

@mikehardy mikehardy commented Sep 20, 2023

Description

There was an older branch I noticed that had two exploratory test cases I used to collaborate with firebase-ios-sdk in order to work through some upstream issues. They are fixed upstream and released now, so we can adopt the tests in our main branch

Patched them up to work with the new modular e2e files since that happened in the interim


Includes 2 random commits that help local testing and don't deserve a separate PR:

  • a detox fix that is backwards-compatible but allows me to test things easily locally now that I've got Xcode 15 / iOS 17 in use in some local environments
  • a java lint fix from a previous PR since android lint silently fails in CI at the moment

Related issues

firebase/firebase-ios-sdk#10370
firebase/firebase-ios-sdk#10463

Release Summary

conventional commits

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan

This was all testing / collaboration (via testing...) with firebase-ios-sdk


Think react-native-firebase is great? Please consider supporting the project with any of the below:

@mikehardy mikehardy added the Workflow: Pending Merge Waiting on CI or similar label Sep 20, 2023
@vercel
Copy link

vercel bot commented Sep 20, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-firebase ❌ Failed (Inspect) Sep 20, 2023 2:30pm
react-native-firebase-next ❌ Failed (Inspect) Sep 20, 2023 2:30pm

this test exposed an upstream bug with a fix that will be released
in firebase-ios-sdk 10.2.0 - good to merge + release after upstream release
iOS 17 emits a slightly different error when there is nothing to terminate

attempting to terminate an app when it is already terminated is not an error,
so it is important for detox to recognize the new string iOS17 uses
or detox startup fails completely
this check silently fails in CI so some things merge despite needing
a re-format - here's a post-merge re-format
@codecov
Copy link

codecov bot commented Sep 20, 2023

Codecov Report

Merging #7361 (e57c7ee) into main (80ac07e) will decrease coverage by 14.92%.
Report is 3 commits behind head on main.
The diff coverage is 8.34%.

Additional details and impacted files
@@              Coverage Diff              @@
##               main    #7361       +/-   ##
=============================================
- Coverage     68.91%   53.99%   -14.92%     
- Complexity        0      736      +736     
=============================================
  Files           137      239      +102     
  Lines          5708    12013     +6305     
  Branches       1224     1886      +662     
=============================================
+ Hits           3933     6485     +2552     
- Misses         1680     5180     +3500     
- Partials         95      348      +253     

@mikehardy mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label Sep 20, 2023
@mikehardy mikehardy merged commit 6f77c19 into main Sep 20, 2023
15 of 17 checks passed
@mikehardy mikehardy deleted the @mikehardy/storage-secondary-app-no-events branch September 20, 2023 15:52
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.

1 participant