Skip to content

test(indy-vdr): add delay after DID creation #1406

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

Merged

Conversation

genaris
Copy link
Contributor

@genaris genaris commented Mar 25, 2023

In Indy VDR tests, after DID creation we are attempting to resolve the recently created DID immediately, which does not always work properly in CI. Here we add the 1000 ms delay that we also do in other Indy registrar tests.

Signed-off-by: Ariel Gentile <gentilester@gmail.com>
@genaris genaris requested a review from a team March 25, 2023 11:57
@codecov-commenter
Copy link

codecov-commenter commented Mar 25, 2023

Codecov Report

Merging #1406 (98f9e13) into main (996c08f) will increase coverage by 0.44%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1406      +/-   ##
==========================================
+ Coverage   84.90%   85.34%   +0.44%     
==========================================
  Files         788      788              
  Lines       19445    19445              
  Branches     3162     3162              
==========================================
+ Hits        16509    16595      +86     
+ Misses       2929     2843      -86     
  Partials        7        7              

see 3 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@@ -60,5 +61,8 @@ export async function createDidOnLedger(agent: Agent, submitterDid: string) {
)
}

// Wait some time pass to let ledger settle the object
await sleep(1000)
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be better to see if we can resolve it? This would still make it non-deterministic albeit less.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you elaborate a bit more on this suggestion? Do you mean to do a call to dids.resolve() before leaving this method?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Resolve it on a retry or something so we know whether it for sure registered or not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the idea. The sleep was an easy workaround but now that it's spread in lots of places makes sense to make a specific function for this problem.

@TimoGlastra TimoGlastra merged commit 3bff26d into openwallet-foundation:main Mar 29, 2023
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.

4 participants