Skip to content

add tests for PythAccount and PythClient #9

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

add tests for PythAccount and PythClient #9

wants to merge 4 commits into from

Conversation

cctdaniel
Copy link
Contributor

No description provided.

@cctdaniel cctdaniel requested a review from SEJeff November 29, 2021 12:11
@cctdaniel cctdaniel self-assigned this Nov 29, 2021
SEJeff
SEJeff reviewed Nov 29, 2021
View reviewed changes
Copy link
Contributor

@SEJeff SEJeff left a comment

Choose a reason for hiding this comment

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

Excellent start! Some nits and some suggestions

tests/test_pyth_client.py
Comment on lines +24 to +87
BCH_PRODUCT_ACCOUNT_KEY_DEVNET = '89GseEmvNkzAMMEXcW9oTYzqRPXTsJ3BmNerXmgA1osV'
BCH_PRICE_ACCOUNT_KEY_DEVNET = '4EQrNZYk5KR1RnjyzbaaRbHsv8VqZWzSUtvx58wLsZbj'

MAPPING_ACCOUNT_B64_DATA = ('1MOyoQIAAAABAAAAWAAAAAEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABqIGcc'
'Dj+MshnOP0blrglqTy/fk20r1NqJJfcAh9Ud2A==')
PRODUCT_ACCOUNT_B64_DATA = ('1MOyoQIAAAACAAAAlwAAADAClHlZh5cpDjY4oXEsKb3iNn0OynlPd4sltaRy8ZLeBnN5bWJvbAdCQ0gv'
'VVNECmFzc2V0X3R5cGUGQ3J5cHRvDnF1b3RlX2N1cnJlbmN5A1VTRAtkZXNjcmlwdGlvbgdCQ0gvVVNE'
'DmdlbmVyaWNfc3ltYm9sBkJDSFVTRARiYXNlA0JDSA==')
PRICE_ACCOUNT_B64_DATA = ('1MOyoQIAAAADAAAAEAsAAAEAAAD3////GwAAAAIAAAAfPsYFAAAAAB4+xgUAAAAA0B+GxYIAAAB/xYqq'
'AAAAADy4oy8BAAAAtPuFGgAAAAC8tR2HAAAAADy4oy8BAAAAAQAAAAAAAAAAAAAAAAAAAGogZxwOP4yy'
'Gc4/RuWuCWpPL9+TbSvU2okl9wCH1R3YAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAdPsYF'
'AAAAACB1G8yCAAAASJxHLgAAAAAvuzCkNjwQn8DZCsmCAAAASCynLwAAAAABAAAAAAAAAB8+xgUAAAAA'
'Qlxb88UapZ0T6mWzABhtX/lDiPrAaUMbsl4vmXpBgd4AI6GaggAAAICFtQ0AAAAAAQAAAAAAAAAdPsYF'
'AAAAAIATnJ2CAAAAAJW6CgAAAAABAAAAAAAAAB4+xgUAAAAAopQU2JDnE5ZYJwruatN5x2coYY19zVBC'
'tyZbiKhxYksAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAAkH6Erg47Ci94ZGUUYRxRICzgpNlfaIVeXC3eoge5K66z0OUXhQAAANMiBSEAAAAA'
'AQAAAAAAAABqYa8FAAAAALPQ5ReFAAAA0yIFIQAAAAABAAAAAAAAAGphrwUAAAAAJyCj9u7+AUmDcI1H'
'T9GvlDfStBYeQB5YYZZZsDQht+AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAHkXfq/XM16tovM/VgEWSsYzNZhi/J9PvVgRbUtqgcg8dWmr8'
'ggAAADjoaxIAAAAAAQAAAAAAAAAcPsYFAAAAAIn1Af2CAAAAi/1rEgAAAAABAAAAAAAAAB0+xgUAAAAA'
'E3YWCAU39ntOTBHgm48UMpFVs7DvUTFB/bbaq/vZeC4AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAXp50gnYmYi6R4+I+QnSWi4H88VJKoIye'
'5Uqoweh9l+UAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAAZk/hVoPBHzoEnahGCoRrjFSg5bWEvDQW7clr70r1C8UAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAquLwEjnZE1h1qKp9'
'mpfiZ+fqJ1Rw3cLhX8nrx0VgKfEA599MkQAAAAB2sBAAAAAAAQAAAAAAAADCe1wFAAAAAADn30yRAAAA'
'AHawEAAAAAABAAAAAAAAAMJ7XAUAAAAANzRkq/Fg5DGQKTxjG3GJaaHanmhr9krIb1OThq282nAAAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'dkkNS7shgAYOa/R2+DNwiO1TuFMlP/ht6SdRU3x62T0AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAcTBWfR0upQv8cz+gPNHNt0GPnKBaObi9'
'LAKefxb5wtsAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAA1FflJlHKD2zpeTMZ6awJhRePbFADBPK0iyu32DjydxMAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQ4KPo2Gdpryu1okX'
'3h18zpIX3scrrhIwY/97590vlj7AoFlfkAAAAMDXGTAAAAAAAQAAAAAAAACokB0FAAAAAMCgWV+QAAAA'
'wNcZMAAAAAABAAAAAAAAAKiQHQUAAAAAf7Bx65O+Q/eDp7AcK8Lw03LmNh99Mpfu5nsydLpn2MUAAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'GxE1Ex2rDAJyecg2P8ogqo9tPGKaDqZCf0+OHUfLz/EIHZd/hAAAALmp0SQAAAAAAQAAAAAAAADBxLcF'
'AAAAAAgdl3+EAAAAuanRJAAAAAABAAAAAAAAAMHEtwUAAAAAskWdp1YAT2k7uotwDoVkx9WSY7gay8uo'
'ykAsyQ6FrusAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAArq/eOgx1mU6Ixigj8cizk6fgfgXNTYnoHl9La1kz/CeAeCunjgAAAIDLeDEAAAAA'
'AQAAAAAAAABMtDcFAAAAAIB4K6eOAAAAgMt4MQAAAAABAAAAAAAAAEy0NwUAAAAA5v6vZs5/Kw4Sf3Gf'
'jLwRpg0NfHtESw5mRqECMnlwNLsAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAZFIJBu0qw/EJSssd8apY//Qv+Wl5hRi697NmiqraVk0AAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAADhxFoFAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAOHEWgUAAAAA'
'jmNjneYqRRu77K9pGMTM31Dr7Hq6X3CAdoTOnMfxvKYAL4BmjwAAAIDR8AgAAAAAAQAAAAAAAAAch0YF'
'AAAAAAAvgGaPAAAAgNHwCAAAAAABAAAAAAAAAByHRgUAAAAAU8TtEyg6nKJ630rA85k7MfQylqJgcsND'
'5LnQsZ7hpLurmVVTjwAAAADC6wsAAAAAAQAAAAAAAADMjEYFAAAAAKuZVVOPAAAAAMLrCwAAAAABAAAA'
'AAAAAMyMRgUAAAAAmNhsxmReIdbBhxZ8WjXWTYNiwcVJPqEt0UqCDDpH5XYTB71SjwAAAAAcTg4AAAAA'
'AQAAAAAAAADcjEYFAAAAABMHvVKPAAAAABxODgAAAAABAAAAAAAAANyMRgUAAAAATvf0GXTayRqQxVor'
'MaSVHlP7Byc52vz8WQF/b2TeHGsvK9hRjwAAAIDfFxAAAAAAAQAAAAAAAADcjEYFAAAAAC8r2FGPAAAA'
'gN8XEAAAAAABAAAAAAAAANyMRgUAAAAADcMZLVXjwPNi+/UPw5fcAP5p0QpTeiI4ES5mWhy7pWQAAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAADcjEYFAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAANyMRgUAAAAA'
'YW9iNZroU1iQRzOa1Eib/co4u8Na3Sv0XL7nFtovGb8AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAASMRtW9UzBCNs7+OA+tYXWIAuPKUTI+uG'
'9WgYebtjLGYAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
'AAAAAAAAAAAAAAAA')
Copy link
Contributor

Choose a reason for hiding this comment

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

Make these into fixtures please

tests/test_pyth_account.py
Comment on lines +9 to +14
BCH_PRODUCT_ACCOUNT_KEY_DEVNET = '89GseEmvNkzAMMEXcW9oTYzqRPXTsJ3BmNerXmgA1osV'
BCH_PRICE_ACCOUNT_KEY_DEVNET = '4EQrNZYk5KR1RnjyzbaaRbHsv8VqZWzSUtvx58wLsZbj'

PRODUCT_ACCOUNT_B64_DATA = ('1MOyoQIAAAACAAAAlwAAADAClHlZh5cpDjY4oXEsKb3iNn0OynlPd4sltaRy8ZLeBnN5bWJvbAdCQ0gv'
'VVNECmFzc2V0X3R5cGUGQ3J5cHRvDnF1b3RlX2N1cnJlbmN5A1VTRAtkZXNjcmlwdGlvbgdCQ0gvVVNE'
'DmdlbmVyaWNfc3ltYm9sBkJDSFVTRARiYXNlA0JDSA==')
Copy link
Contributor

Choose a reason for hiding this comment

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

Make these into fixtures

tests/test_pyth_client.py
watch_session: WatchSession
) -> None:
ws = pyth_client.create_watch_session()
assert ws._next_subid() == watch_session._next_subid()
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this actually supposed to be testing? Is it possible to setup something so you test the public methods here?

This definitely expands the coverage, but I'm not sure what this is actually asserting other than the two subscription ids are the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the purpose is to purely test the create_watch_session() function, with regards to the actual WatchSession functions, it could be done on a separate file?

Copy link
Contributor Author

@cctdaniel cctdaniel Nov 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

what do you think about testing if this function returns an instance of WatchSession?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok, yeah that makes sense. The isinstance thing is a good idea here. I just wasn't quite sure what this was supposed to be doing, thanks for clarifying.

As an example, for the binary tests in #3, I was doing 1 test file for each of the big classes in pythclient.pythaccounts instead of 1 test file per source file. It makes it a little more organized that way. I try to make smaller test files when feasible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup, I'm following your style but the create_watch_session() is in the PythClient class and there's a separate WatchSession class although both of them are in pythclient.py

tests/test_solana_account.py
Comment on lines +32 to +33
assert solana_account.slot is None
assert solana_account.lamports is None
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
assert solana_account.slot is None
assert solana_account.lamports is None

Unless it is necessary for the logic, I think that testing the fixtures outside of the fixture code doesn't make much sense. wdyt?

Copy link
Contributor Author

@cctdaniel cctdaniel Nov 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

agree, will remove

@cctdaniel cctdaniel mentioned this pull request Dec 1, 2021
Merged
@cctdaniel
Copy link
Contributor Author

addressed issues in #11 -- closing this PR because branch is unlinked due to fork associations being broken when turning private repo to public

@cctdaniel cctdaniel closed this Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SEJeff SEJeff SEJeff left review comments

Assignees

@cctdaniel cctdaniel

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cctdaniel @SEJeff