fixes MissingDelegableApprovalError logging bug #789
Conversation
Signed-off-by: 2byrds <2byrds@gmail.com>
| @@ -739,6 +739,8 @@ def test_delegables_escrow(): | |||
| parsing.Parser().parse(ims=bytearray(gateIcp), kvy=torKvy, local=True) | |||
| assert gateHab.pre not in torKvy.kevers | |||
| assert len(torHab.db.delegables.get(keys=snKey(gateHab.kever.serder.preb, gateHab.kever.serder.sn))) == 1 | |||
| # Exercise the MissingDelegableApprovalError case | |||
| torKvy.processEscrowDelegables() | |||
There was a problem hiding this comment.
Is there expected to be additional assertions here to catch MissingDelegableApprovalError?
There was a problem hiding this comment.
or should there be additional assertions to verify there wasn't a missing delegable approval error?
There was a problem hiding this comment.
seems odd to add the processing line but no assertions in the test
There was a problem hiding this comment.
Is there expected to be additional assertions here to catch
MissingDelegableApprovalError?
No, the error is part of the escrow processing (the exception doesn't make it out of the processing).
There was a problem hiding this comment.
or should there be additional assertions to verify there wasn't a missing delegable approval error?
The test was valid originally, but it never exercised the MissingDelegableApprovalError so it didn't discover the logging error. This small addition does a better job of simulating that processEscrowDelegables is being called multiple times during the flow (becuase it is part of asynchronous escrow processing)
There was a problem hiding this comment.
seems odd to add the processing line but no assertions in the test
The constantly running escrow processors are difficult to capture in a small test. this is the shortest path to exercising the case when the processor runs before the user managed flow is complete.
| @@ -5806,7 +5806,7 @@ def processEscrowDelegables(self): | |||
| self.processEvent(serder=eserder, sigers=sigers, wigers=wigers, delseqner=seqner, | |||
| delsaider=saider, local=esr.local) | |||
| else: | |||
| raise MissingDelegableApprovalError() | |||
| raise MissingDelegableApprovalError("No delegation seal found for event.") | |||
There was a problem hiding this comment.
might be good to include the event digest?
MissingDelegableApprovalError("Missing Delegable Approval for evt at dig = {}.""".format(bytes(edig)))
There was a problem hiding this comment.
Yes, that would be better
Adds a message to the MissingDelegableApprovalError so the logging doesn't fail.
Updated test to exercise the exception and logging.