Skip to content

Conversation

@srushtisv
Copy link
Contributor

Implement integration test for verifying phone number sign-in with Firebase Auth. Please review and approve.

@srushtisv srushtisv changed the title Implement integration test for verifying phone number sign-in with Firebase Auth. Please review and approve. Adding Integration Test for Phone Number Authentication in Firebase Aug 23, 2024
@google-oss-bot
Copy link

1 Warning
⚠️ Did you forget to add a changelog entry? (Add #no-changelog to the PR description to silence this warning.)

Generated by 🚫 Danger

@paulb777
Copy link
Member

LLC is missing. See other files in the repo or https://github.com/firebase/firebase-ios-sdk/blob/main/scripts/check_copyright.sh#L21

Copy link
Member

@paulb777 paulb777 left a comment

Choose a reason for hiding this comment

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

Thanks for working on tests. The new file should be added to the SwiftApiTests target in the Xcode project so it builds and runs.

When I do that locally, it has some failures.

Also, it would be great to test the async variations of the APIs as well.

Copy link
Member

@ncooke3 ncooke3 left a comment

Choose a reason for hiding this comment

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

The check CI can be fixed by running:

scripts/style.sh FirebaseAuth/Tests/SampleSwift/SwiftApiTests/PhoneAuthTest.swift

You can also run it without args though, but I find it sometimes slows when it checks all the source. I like to run git diff --name-only main | xargs scripts/style.sh to style everything on my branch different from main.

@srushtisv srushtisv force-pushed the sv-PhoneAuth-test1 branch 2 times, most recently from 66252e9 to 1dde83b Compare August 29, 2024 01:49
Copy link
Member

@ncooke3 ncooke3 left a comment

Choose a reason for hiding this comment

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

Since the plist wasn't changed in this PR, I think it makes sense to revert all xcode proj changes that mention the plists

@ncooke3
Copy link
Member

ncooke3 commented Aug 30, 2024

Thanks for working on tests. The new file should be added to the SwiftApiTests target in the Xcode project so it builds and runs.

When I do that locally, it has some failures.

Also, it would be great to test the async variations of the APIs as well.

confirmed they are building and running:

PhoneAuthTests
    ✓ testSignInWithPhoneNumber (0.997 seconds)
    ✓ testSignInWithPhoneNumberAsync_Success (1.066 seconds)

from https://github.com/firebase/firebase-ios-sdk/actions/runs/10625088625/job/29454486166?pr=13538#step:6:340

@srushtisv
Copy link
Contributor Author

Thanks for working on tests. The new file should be added to the SwiftApiTests target in the Xcode project so it builds and runs.

When I do that locally, it has some failures.

Also, it would be great to test the async variations of the APIs as well.

done

@srushtisv srushtisv closed this Aug 30, 2024
@srushtisv srushtisv deleted the sv-PhoneAuth-test1 branch August 30, 2024 19:54
@srushtisv srushtisv restored the sv-PhoneAuth-test1 branch August 30, 2024 19:54
@srushtisv srushtisv reopened this Aug 30, 2024
@srushtisv srushtisv requested a review from paulb777 August 30, 2024 20:16
@pragatimodi pragatimodi self-requested a review September 4, 2024 16:21
@srushtisv srushtisv changed the base branch from main to rce-phone-tests September 10, 2024 10:17
@paulb777
Copy link
Member

@srushtisv Any reason this code hasn't yet merged to main? Ideally, we should merge larger PRs to main as soon as possible to avoid unnecessary branching.

@srushtisv srushtisv changed the base branch from rce-phone-tests to main September 17, 2024 08:05
@srushtisv srushtisv changed the base branch from main to rce-phone-tests September 17, 2024 08:06
@srushtisv srushtisv changed the base branch from rce-phone-tests to main September 17, 2024 08:07
@srushtisv srushtisv changed the base branch from main to rce-phone-tests September 17, 2024 08:08
@srushtisv srushtisv changed the base branch from rce-phone-tests to main October 15, 2024 09:15
@srushtisv srushtisv changed the base branch from main to rce-phone-tests October 15, 2024 09:15
@srushtisv srushtisv closed this Dec 6, 2024
@firebase firebase locked and limited conversation to collaborators Jan 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants