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

Upgrade anoncreds to 0.2.0.dev7 #2719

Merged
merged 5 commits into from
Jan 18, 2024
Merged

Conversation

jamshale
Copy link
Contributor

@jamshale jamshale commented Jan 17, 2024

Used an exact version of the dev pre-release. Figured is would be changed anyway when this becomes non pre-release.

Fixed some calls to the anoncreds library. Required fetching some additional objects from the wallet. Fixed the unit tests.

These would have been quicker and easier to fix if the unit tests caught them. For some of the unit tests I was able to interact with anoncreds and create objects with it, but for some objects like creating credentials and revocation registries I was mocking the objects. These ended up being the broken calls so only the integration tests caught them.

Would be nice if the unit tests could create all the anoncreds objects instead of mocking them. Will try and do this more when I see the opportunity.

Changed the devcontainer poetry version to match the version used to upgrade.

Signed-off-by: jamshale <jamiehalebc@gmail.com>
@swcurran swcurran requested review from dbluhm and ianco January 17, 2024 18:37
swcurran
swcurran previously approved these changes Jan 17, 2024
@jamshale
Copy link
Contributor Author

Integration tests failing. Looking into them.

@jamshale
Copy link
Contributor Author

More things to fix here then I thought there would be. Quite a few changes to anoncreds object create methods. Some of the complex objects I wasn't able to unit test well, like creating credentials and some revocation objects. These are the failing ones.

@swcurran
Copy link
Member

Sorry this is a bigger than expected task, but I think it is the priority, so this is good for you to work through.

It wasn’t until I saw your PR that I realized we weren’t on the 0.2.0 path. Getting on that is necessary.

@swcurran swcurran self-requested a review January 17, 2024 19:40
@swcurran swcurran dismissed their stale review January 17, 2024 19:41

Provided too early...

Signed-off-by: jamshale <jamiehalebc@gmail.com>
@jamshale
Copy link
Contributor Author

This should be good now. All the integration tests pass. Updated the description comment.

@jamshale jamshale marked this pull request as ready for review January 17, 2024 22:58
@ianco
Copy link
Member

ianco commented Jan 18, 2024

Would be nice if the unit tests could create all the anoncreds objects instead of mocking them. Will try and do this more when I see the opportunity.

Yes this would be nice. Prob best to deal with this strategically since the integration tests test the full stack top-to-bottom.

Copy link

sonarcloud bot commented Jan 18, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@swcurran swcurran merged commit 0e95d01 into hyperledger:main Jan 18, 2024
8 checks passed
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.

None yet

4 participants