Skip to content
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

fix: addressing internal feedback (batch 6) #139

Merged
merged 3 commits into from
Feb 14, 2025
Merged

Conversation

aorumbayev
Copy link
Contributor

@aorumbayev aorumbayev commented Feb 13, 2025

Proposed Changes

  • ensure root of unnamed resources response is parsed into typed class (which is a root cause of an error reported by Brian)
  • align error message thrown during errors in resource population with ts (reported by Neil)

Copy link

github-actions bot commented Feb 13, 2025

Coverage

Coverage Report
FileStmtsMissCoverMissing
src/algokit_utils
   _debugging.py1411887%21, 40–42, 45, 54, 62, 81, 87, 96, 110–114, 133, 141–143
   account.py330%1–12
   algorand.py981585%62–63, 74–75, 95–96, 108–110, 119–120, 214, 234, 247, 263
   application_client.py330%1–11
   application_specification.py770%1–39
   config.py782173%23, 30–34, 73–74, 90, 95, 100, 111–116, 141, 144–146, 149, 151
   deploy.py330%1–10
   logic_error.py330%1–10
src/algokit_utils/_legacy_v2
   _ensure_funded.py71199%100
   _transfer.py70396%14, 75–76
   account.py931386%15–18, 70–74, 109, 126, 156, 159, 203
   application_client.py5377786%56–57, 176, 181, 210, 322, 327–328, 330, 332, 801, 816, 834–837, 931, 971, 983, 996, 1038, 1098–1104, 1108–1113, 1115, 1151, 1158, 1271, 1307, 1321, 1359–1361, 1363, 1373–1430, 1441–1446, 1466–1469
   asset.py80495%24–27
   common.py13192%13
   deploy.py4072295%32–35, 170, 174–175, 193, 249, 339–340, 361, 395, 406–414, 429, 437, 593–594, 618
   network_clients.py73593%77–78, 101–102, 135
src/algokit_utils/accounts
   account_manager.py2123384%161, 180–181, 205–210, 228–229, 246, 263–268, 315–317, 346, 385, 405–409, 422, 481, 502, 659, 765, 844, 849, 868–869, 892
   kmd_account_manager.py741086%49–55, 96, 152, 159
src/algokit_utils/applications
   abi.py1327146%14, 77, 113, 119, 121, 123, 127–128, 143–162, 178, 181–187, 203–216, 229–241, 256–267
   app_client.py70421470%66–74, 133, 141, 335–338, 341, 344, 347, 350, 362–365, 368, 371, 374, 377, 389, 398, 407, 410–477, 490–548, 572–574, 591–594, 602–605, 613–616, 624–627, 635–638, 649–652, 665, 761–764, 798, 810, 822, 834, 846, 861, 876, 886, 896, 906, 916, 926, 963–976, 992, 1009, 1026, 1043, 1064, 1085, 1175, 1343, 1390–1391, 1422, 1424, 1430, 1475–1483, 1516–1519, 1522–1525, 1546, 1594, 1657–1659, 1672–1679, 1720, 1737, 1739, 1742, 1746, 1873, 1884–1885, 1913, 1923–1925, 1928–1950, 1960, 1969–1984, 1989–1994
   app_deployer.py2143584%98, 196, 203, 213–218, 221–225, 297–298, 507, 516–519, 532, 537, 543, 552–557, 564, 578, 589–630
   app_factory.py2662790%351, 361, 364, 508, 516, 528, 677, 691–696, 702–703, 721, 746, 757–758, 794, 802–816
   app_manager.py2181693%226, 285–286, 318–323, 349, 367–368, 398, 419–422, 445, 454
src/algokit_utils/applications/app_spec
   arc32.py95892%198–207
   arc56.py4703692%72–74, 183, 318, 336–337, 415, 437–439, 495, 504, 506, 574, 769, 779, 783, 943, 945, 978–989, 1002, 1004–1005, 1021
src/algokit_utils/assets
   asset_manager.py1061289%266–267, 276, 282–306, 316
src/algokit_utils/clients
   client_manager.py1735469%24–26, 74–81, 95, 127–129, 185, 196–199, 224, 263, 298–303, 338–340, 373, 390, 412, 446–449, 482–485, 519–522, 552–555, 573–598, 628, 637, 645
   dispenser_api_client.py841286%129–130, 134–137, 172–174, 193–195
src/algokit_utils/errors
   logic_error.py561180%14, 105–121
src/algokit_utils/models
   account.py921584%32, 34, 80–81, 128, 136, 144, 168–175, 188, 196, 209, 217
   amount.py1061784%35, 42, 88, 98, 104, 112, 117–119, 126, 131–133, 140, 147, 160, 166
   application.py42198%8
   state.py36586%51, 55–58
   transaction.py51394%66, 86, 91
src/algokit_utils/protocols
   account.py11282%17, 22
   typed_clients.py24483%12–24
src/algokit_utils/transactions
   transaction_composer.py96611089%34–41, 601, 640, 643, 648, 652–663, 695, 736, 803, 810, 832, 865, 962–991, 1020, 1038–1040, 1046–1061, 1066, 1074, 1076, 1078, 1094, 1100, 1138, 1146, 1159, 1253, 1303, 1552–1553, 1592–1593, 1634, 1845, 1848, 1867, 1872, 1896–1898, 1935–1971, 1978–1985, 2126, 2130, 2261, 2263–2264, 2314–2315
   transaction_creator.py75791%116, 121, 126, 131, 136, 141, 156
   transaction_sender.py1481093%88, 243, 286–287, 431–436, 441–442, 487
TOTAL626391285% 

Tests Skipped Failures Errors Time
402 0 💤 0 ❌ 0 🔥 5m 53s ⏱️

Copy link
Contributor

@neilcampbell neilcampbell left a comment

Choose a reason for hiding this comment

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

I'm curious why the resource population tests didn't flag this as a problem.

@aorumbayev
Copy link
Contributor Author

@neilcampbell we haven't had tests for certain unnamed resources fields from the response such as asset-holding and etc. Hence went unnoticed as main use cases were covered by tests identical to uitls. The change in the pr ensures that entire response body is parsed into a typed object.

@aorumbayev aorumbayev merged commit 4c661c0 into main Feb 14, 2025
4 checks passed
@aorumbayev aorumbayev deleted the fix/feedback-6 branch February 14, 2025 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants