search

bcgov / aries-acapy-plugin-redis-events

Apache License 2.0
5 stars 13 forks source link

Fix outbound transit after acapy upgrade #25

Closed jamshale closed 11 months ago

jamshale commented 11 months ago

After learning about the plugin and how to test it properly I realized the custom outbound transit broke with the acapy upgrade. I think the order of the wire_format and root_profile parameters must have changed and it wasn't using named arguments. I fixed it by copying how the http outbound in acapy was using the base class.

jamshale commented 11 months ago

👍🏻 from me - don't know much about WHY you're encountering the error you found though.

Question: are we planning on executing tests on PR submissions in the future?

Yes. I the plugin repo runs integration tests for changed plugins on every PR. It would have caught this. The failing unit tests should have been an indication that there was a problem as well but I didn't know enough to realize what problem it was causing.

The error was:

integration-faber-1 | Traceback (most recent call last): integration-faber-1 | File "/usr/local/lib/python3.9/asyncio/tasks.py", line 256, in step integration-faber-1 | result = coro.send(None) integration-faber-1 | File "/usr/src/app/.venv/lib/python3.9/site-packages/aries_cloudagent/transport/outbound/manager.py", line 148, in start_transport integration-faber-1 | transport = self.registered_transports[transport_id]( integration-faber-1 | TypeError: init__() missing 1 required positional argument: 'wire_format'

I had noticed it before but didn't realize that the outbound transit was breaking because of it. I still don't understand why removing it from the parent class initializer fixes this error though.

jamshale commented 11 months ago

I could change the integration tests to the ones I'm adding in the plugin repo. I think they are more thorough. One reason I didn't catch this sooner was because the integration tests didn't really tests the outbound transit.