firebase-ios-sdk
firebase-ios-sdk copied to clipboard
Verify that @DocumentID of String works with custom Codable methods.
In one of my apps, I had to deal with a situation that called for implementing custom encoding / decoding. This PR adds tests to verify custom Codable methods work with @DocumentID if the underlying attribute is of type String
.
This is related to #7247
#no-changelog
1 Warning | |
---|---|
:warning: | Did you forget to add a changelog entry? (Add #no-changelog to the PR description to silence this warning.) |
Generated by :no_entry_sign: Danger
This seems unnecessary.
We already have unit tests that test various permutations of @DocumentID
: https://github.com/firebase/firebase-ios-sdk/blob/88ecc3cd5fc55291eca07ccc9a0209fb8c222f0b/Firestore/Swift/Tests/Codable/FirestoreEncoderTests.swift#L574
While these don't specifically cover custom Codable models, these do verify that @DocumentID
does the right thing when wrapping a String
.
Integration tests are far more expensive in terms of run time than unit tests, so the integration test for @DocumentID
only directly covers DocumentID<DocumentReference>
. It shows that @DocumentID
works at all when encountering real document references, without verifying all the different ways it can be used.
The testSelfDocumentIDWithCustomCodable
test I added in #7247 originally started as just trying to figure out how to use DocumentID
in custom Codable implementations. While this test technically does not add any new coverage, this was tricky enough to make work at all that I wanted to leave this in the suite, essentially as an example that shows this is possible. However, once we've delegated to DocumentID
, the existing unit tests cover wrapped type permutations again.
These new tests don't really add any new coverage and do extend suite run time so I'm disinclined toward merging this.
@peterfriese Should we close this one?