Skip to content
This repository has been archived by the owner on Dec 12, 2024. It is now read-only.

Feat/update quote orderstatus #68

Closed
wants to merge 3 commits into from

Conversation

kirahsapong
Copy link
Collaborator

@kirahsapong kirahsapong commented Mar 20, 2024

closes #67

this one's a bit weird because it adds pretty json and escaped json strings to the tests for demonstration purposes - lmk if this is a value add for anyone rn or not

@blackgirlbytes @acekyd

also - rationale for removing init from quote and orderstatus is that these messages should never be initialized from the client, since a client will never send these

protocol: nil
)

private func parsedOrderStatus(orderStatus: String) throws -> OrderStatus? {

Choose a reason for hiding this comment

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

maybe parseOrderStatus to use da verb?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah i also thought verbs are universally clean code for functions. but learned from adam in swift you name them the noun.

}
"""

let orderStatusStringJSON =

Choose a reason for hiding this comment

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

can this be a test vector?

Choose a reason for hiding this comment

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

also, orderStatus: wee 😆

if let orderStatus = try parsedOrderStatus(orderStatus: orderStatusStringJSON) {
XCTAssertEqual(orderStatus.metadata.kind, MessageKind.orderStatus)
} else {
XCTFail("Order status is not a parsed orderStatus")

Choose a reason for hiding this comment

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

i want to understand how you expect this test to fail - are we calling this test function with 1 valid orderstatusJson and expecting it to never drop into the else assertion here?

lining up multiple test cases and running them through tests to ensure what we want to see pass passes and what we want to see fail fails is how we've been approaching it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah i always do it that way. missed it going quickly here.

@kirahsapong
Copy link
Collaborator Author

closing in favour of allowing init on these Messages and Resource types. Details here #67

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow init on Resource types
2 participants