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

Feat/add get balance #77

Merged
merged 7 commits into from
Mar 29, 2024
Merged

Feat/add get balance #77

merged 7 commits into from
Mar 29, 2024

Conversation

kirahsapong
Copy link
Contributor

@kirahsapong kirahsapong commented Mar 26, 2024

closes #56

@kirahsapong kirahsapong force-pushed the feat/add-get-balance branch from f51bf39 to e13c384 Compare March 28, 2024 19:25
@kirahsapong kirahsapong marked this pull request as ready for review March 28, 2024 19:38
@@ -9,7 +9,7 @@ final class tbDEXTestVectorsProtocol: XCTestCase {

// MARK: - Resources

func test_parseOffering() throws {
func _test_parseOffering() throws {

Choose a reason for hiding this comment

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

is there a swift-related reason for the _ prefix here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mentioned - but yeah means skip this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tests must be prefixed with test so really anything would do

@@ -83,6 +83,27 @@ final class tbDEXHttpClientTests: XCTestCase {
XCTAssertNotNil(offerings[0].signature)
}

func test_getBalancesWithOneValidBalance() async throws {

Choose a reason for hiding this comment

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

could we add all the failure test cases similar to what you did in TBD54566975/tbdex-js#212 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

some of these tests end up testing a ton of duplicative functionality, so was planning to refactor the source code into a shared private function (made a note at top of the file). wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

opened an issue to track here - #85

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added mo!

return parsedBalance
}

let balancePrettyJSON = """

Choose a reason for hiding this comment

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

ah, maybe _ prefix means they are skipped? i am inferring this test suite is wip

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes thats right 👍 good intuition

@kirahsapong kirahsapong merged commit 6476eb1 into main Mar 29, 2024
6 checks passed
@kirahsapong kirahsapong deleted the feat/add-get-balance branch March 29, 2024 18:59
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.

Implement GET account balance client method
2 participants