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

Aca-Py test scenario including a container restart (with aca-py version upgrade) #3400

Merged
merged 9 commits into from
Dec 17, 2024

Conversation

ianco
Copy link
Contributor

@ianco ianco commented Dec 13, 2024

Test for Issue #3269.

Script starts agents using docker-compose - alice agent is based on the Aca-Py 1.0.1 release image. Alice connects with and then issues a credential to bob and then the test shuts down the alice agent, restarts based on the acapy-test image (based on the local aca-py codebase) and then asks for a proof.

This is working! I'm going to release this PR as a simple test-with-restart and do a separate test with an askar-to-anoncreds wallet upgrade.

@ianco ianco marked this pull request as draft December 13, 2024 01:25
@jamshale
Copy link
Contributor

I believe you can add assertions into the example.py script. Basically if it exits with exit code 1 it fails and if it exists with exit code 0 it passes. There's some caveats with this though. I had a test exiting with code 0 locally but it failed with github actions because the acapy-controller library never received an event it was expecting.

If the example raises an exception then it will fail.

@ianco ianco changed the title WIP aca-py test scenario including a container restart (with aca-py upgrade) Aca-Py test scenario including a container restart (with aca-py version upgrade) Dec 17, 2024
@ianco ianco requested review from jamshale and dbluhm December 17, 2024 16:27
@ianco ianco marked this pull request as ready for review December 17, 2024 16:27
Signed-off-by: Ian Costanzo <[email protected]>
Copy link
Contributor

@jamshale jamshale left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Some of those docker attrs are pretty complicated and might be fragile to upgrading the docker package, but upgrading it won't be necessary for a testing project so that ok.

@ianco
Copy link
Contributor Author

ianco commented Dec 17, 2024

Some of those docker attrs are pretty complicated and might be fragile to upgrading the docker package, but upgrading it won't be necessary for a testing project so that ok.

Yah I agree. For some of the docker stuff it was a bit of trial and error so I'm not sure how fragile these tests are going to be :-S

Copy link
Contributor

@dbluhm dbluhm left a comment

Choose a reason for hiding this comment

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

I'm interested in potentially pulling some of that docker management complexity into an extra of the acapy_controller library in the future. That could be a convenient way to develop more of this style of test. As it stands, I think this is a great addition and I'm eager to get this kind of testing into the pipeline.

@ianco ianco enabled auto-merge (squash) December 17, 2024 21:32
@ianco
Copy link
Contributor Author

ianco commented Dec 17, 2024

I'm interested in potentially pulling some of that docker management complexity into an extra of the acapy_controller library in the future. That could be a convenient way to develop more of this style of test.

Agree. I've started a second test (to include an askar-to-anoncreds) upgrade. For now I'll refactor the common stuff into a util.py (in acapy repo) but agree this should eventually be a feature of the acapy_controller.

@ianco ianco merged commit 969b9f9 into openwallet-foundation:main Dec 17, 2024
9 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.

3 participants