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

Anoncreds-rs pytest update #2541

Conversation

usingtechnology
Copy link
Contributor

@usingtechnology usingtechnology commented Oct 9, 2023

Updates to pytests that were affected by anoncreds-rs updates.

Since we have not updated/implemented endorser in the anoncreds-rs branch, I have left endorser related tests as skipped. These are now marked:

@pytest.mark.skip(reason="anoncreds-rs/endorser breaking change")

closes #2433

Signed-off-by: Jason Sherman <tools@usingtechnolo.gy>
Signed-off-by: Jason Sherman <tools@usingtechnolo.gy>
@usingtechnology usingtechnology requested a review from dbluhm October 9, 2023 20:20
@usingtechnology usingtechnology added the AnonCreds Ledger Agnostic AnonCreds label Oct 16, 2023
@usingtechnology usingtechnology self-assigned this Oct 16, 2023
Signed-off-by: Jason Sherman <tools@usingtechnolo.gy>
Signed-off-by: Jason Sherman <tools@usingtechnolo.gy>
@@ -198,7 +198,7 @@ async def register_schema(
if not profile.settings.get_value("wallet.type"):
# TODO is this warning necessary?
reason += ": missing wallet-type?"
raise AnonCredsRegistrationError(reason)
raise AnonCredsResolutionError(reason)
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like this really ought to be a registration error rather than a resolution error; the operation attempted is a register rather than a resolve so, even in the event we were attempting to resolve something in order to register something immediately after, I think it is better for this to be a registration error from an interface perspective.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, sounds good. This was just inconsistent with other similar calls/logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated related code

@@ -220,7 +222,7 @@ async def create_and_register_schema(
raise AnonCredsIssuerError(
"Schema already exists but was not in wallet; stored in wallet"
) from err
except AnoncredsError as err:
except (AnoncredsError, BaseAnonCredsError, LedgerError) as err:
Copy link
Contributor

Choose a reason for hiding this comment

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

LedgerError will only be raised by Indy components; LedgerError should be wrapped into a BaseAnonCredsError subtype rather than be handled in this abstract/general interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated related code

@@ -335,7 +337,7 @@ async def create_and_register_credential_definition(
CredDef.from_native(cred_def),
options,
)
except AnoncredsError as err:
except (AnoncredsError, BaseAnonCredsError, LedgerError) as err:
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as previous comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated related code.

Signed-off-by: Jason Sherman <tools@usingtechnolo.gy>
@usingtechnology usingtechnology changed the title Anoncreds-rs pytest update (Credential Definitions) Anoncreds-rs pytest update Oct 18, 2023
…egacyIndyRegistry

Signed-off-by: Jason Sherman <tools@usingtechnolo.gy>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@dbluhm dbluhm merged commit 28f2d71 into openwallet-foundation:anoncreds-rs Oct 20, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AnonCreds Ledger Agnostic AnonCreds
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants