-
Notifications
You must be signed in to change notification settings - Fork 26
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
Add contentMediaType for variants object #47
Conversation
This is my bad; I should really have asked for tests in your other PR(s)! If you check out the tests folder you should be able to copy one for testing these changes by making sure accessing properties works as expected. It's based on the Twitter v2 OpenAPI spec which they use for their own playground development and some docs generation. |
Oh got it. Will look into that today and add them in 🙌 |
So not really sure how to test this inside an array tbh. I guess you were referring to one of these tests? func testAddListMember() async throws {
let addListMemberResult = try await TwiftTests.userAuthClient.addListMember("0", to: "0")
XCTAssertTrue(addListMemberResult.data.isMember)
} All I can see that I can do to test if the types are there is this. But can only check if they are accessible from within so not sure if that helps testing the final class TwiftTypesTests: XCTestCase {
func testMediaVariantsBitrateAccessible() async throws {
let getTweetResult = try await TwiftTests.userAuthClient.getTweet("0")
XCTAssertNil(getTweetResult.includes?.media?.first?.variants?.first?.bitRate)
}
func testMediaVariantsContentTypeAccessible() async throws {
let getTweetResult = try await TwiftTests.userAuthClient.getTweet("0")
XCTAssertNil(getTweetResult.includes?.media?.first?.variants?.first?.contentType)
}
func testMediaVariantsUrlAccessible() async throws {
let getTweetResult = try await TwiftTests.userAuthClient.getTweet("0")
XCTAssertNil(getTweetResult.includes?.media?.first?.variants?.first?.url)
}
} Or did you mean for me to instantiate a mock media object? |
Oh I see, you mean testing the access levels! Yeah I think since Twift is annotated with |
Sources/Types+Media.swift
Outdated
@@ -95,7 +104,7 @@ extension Media { | |||
public var bitRate: Int? | |||
|
|||
/// Type of media | |||
public var contentType: MediaType | |||
public var contentType: ContentMediaType |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should probably just conform to UTType
!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow...first time I saw this existed 😅 Seems convenient. It covers all "standard" extensions/types basically?
Right! Inside the demo app it is 👌 to test, but not sure how I can write actual tests to see if the data is there? Another note, just tested it with the latest UTType change, it doesn't work. But it does with the previous |
@roblack yeah I noticed UTType doesn't work as expected here; I think because the decoding initialiser expects an identifier(e.g. "public.mpeg4") instead of a MIME type (e.g. "video/mp4"). It may make sense to just decode |
Unfortunately it looks like Twitter's OpenAPI spec doesn't make this as easy as I had hoped either. Basically, it only synthesises some types correctly. Here's an example payload from {
"data": {
"author_id": "2244994945",
"created_at": "Wed Jan 06 18:40:40 +0000 2021",
"id": "1346889436626259968",
"text": "Learn how to use the user Tweet timeline and user mention timeline endpoints in the Twitter API v2 to explore Tweet\\u2026 https:\\/\\/t.co\\/56a0vZUx7i"
},
"errors": [
{
"detail": "string",
"status": 0,
"title": "string",
"type": "string"
}
],
"includes": {
"media": [
{
"height": 0,
"media_key": "string",
"type": "string",
"width": 0
}
],
"places": [
{
"contained_within": [
"f7eb2fa2fea288b1"
],
"country": "United States",
"country_code": "US",
"full_name": "Lakewood, CO",
"geo": {
"bbox": [
-105.193475,
39.60973,
-105.053164,
39.761974
],
"geometry": {
"coordinates": [
-105.18816086351444,
40.247749999999996
],
"type": "Point"
},
"properties": {},
"type": "Feature"
},
"id": "f7eb2fa2fea288b1",
"name": "Lakewood",
"place_type": "city"
}
],
"polls": [
{
"duration_minutes": 5,
"end_datetime": "2019-08-24T14:15:22Z",
"id": "1365059861688410112",
"options": [
{
"label": "string",
"position": 0,
"votes": 0
},
{
"label": "string",
"position": 0,
"votes": 0
}
],
"voting_status": "open"
}
],
"topics": [
{
"description": "All about technology",
"id": "string",
"name": "Technology"
}
],
"tweets": [
{
"author_id": "2244994945",
"created_at": "Wed Jan 06 18:40:40 +0000 2021",
"id": "1346889436626259968",
"text": "Learn how to use the user Tweet timeline and user mention timeline endpoints in the Twitter API v2 to explore Tweet\\u2026 https:\\/\\/t.co\\/56a0vZUx7i"
}
],
"users": [
{
"created_at": "2013-12-14T04:35:55Z",
"id": "2244994945",
"name": "Twitter Dev",
"protected": false,
"username": "TwitterDev"
}
]
}
} As you can see in the above |
Unfortunately I was not able to find proper spec in the Twitter API for this before, and only way to really test this is to have it in the package and use it.
Or could we somehow test this in the demo app @daneden ?
As for this PR, I presumed that mediaType and contentType are the same, however from a bit of testing it turns you types slightly differ in strings, so we needed a new enum to handle that.