Skip to content

fix integration metagraph test#2613

Merged
basfroman merged 5 commits intostaging-pre-merge-port-raofrom
feat/roman/last-integration-test
Jan 30, 2025
Merged

fix integration metagraph test#2613
basfroman merged 5 commits intostaging-pre-merge-port-raofrom
feat/roman/last-integration-test

Conversation

@basfroman
Copy link
Collaborator

@basfroman basfroman commented Jan 30, 2025

we need to improve this test. but it's fine for now

@basfroman basfroman self-assigned this Jan 30, 2025
@basfroman basfroman added testing rao RAO related changes. labels Jan 30, 2025

all_delegates_details = {}
async for ss58_address, identity in identities_info:
for ss58_address, identity in identities_info:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is important

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Exactly so. In this code, the use of async loop is redundant which complicates testing and that's it. Inside this loop there are sync function calls.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it redundant?

Copy link
Contributor

@thewhaleking thewhaleking Jan 30, 2025

Choose a reason for hiding this comment

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

Take this snippet for example:

async def main():
    async with AsyncSubtensor("test") as subtensor:
        print(await subtensor.get_delegate_identities())

It won't work, because AsyncQueryMapResult has no __next__ method.
(with the current async-substrate-interface

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, yeah. This is AsyncQueryMapResult with async iterator instead of QueryMapResult with sync one.
Make sense. I checked all calls inside for loop, but not the iterated object. Let's bring it back and replace mocked object with AsyncMock

@basfroman basfroman merged commit ab673a3 into staging-pre-merge-port-rao Jan 30, 2025
15 checks passed
@basfroman basfroman deleted the feat/roman/last-integration-test branch January 30, 2025 08:03
@ibraheem-abe ibraheem-abe mentioned this pull request Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rao RAO related changes. testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants