Remove in memory wallet #3311
Merged
Remove in memory wallet #3311
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
jamshale
commented
Oct 24, 2024
•
jamshale
commented
- Removes the in-memory wallet and updates tests to use an in-memory sqlite askar wallet.
- Doesn't flatten the wallet abstraction in this PR. Will do this in an additional PR.
- Runs the tests in parallel to speed up execution time.
- Updates all tests that created an in-memory (non-async) profile (wallet) to use the async askar wallet.
- test_dispatcher had a few tests that are skipped. It seems like there might be bug in this code preventing them from working.
- there was one problematic test in test_conductor that I skipped. It has a bad mock somewhere and was causing other tests to fail when run sequentially.
jamshale
force-pushed
the
remove-in-mem-wallet
branch
4 times, most recently
from
d1ef4be to
f9f1c41
Compare
Signed-off-by: jamshale <[email protected]>
Signed-off-by: jamshale <[email protected]>
Signed-off-by: jamshale <[email protected]>
Signed-off-by: jamshale <[email protected]>
Signed-off-by: jamshale <[email protected]>
Signed-off-by: jamshale <[email protected]>
jamshale
force-pushed
the
remove-in-mem-wallet
branch
from
f9f1c41 to
7dc00b9
Compare
Signed-off-by: jamshale <[email protected]>
|
|
Switching to a raw key dramatically sped up the tests. Whole PR Tests take less than 3 minutes now. |
|
This should be ready. It is a breaking change for external plugins that use the old in_memory_profile for testing. |
|
@jamshale if a wallet type is not specified on startup, do we spin up with an in-memory askar wallet? (if so, this should also probably be using raw key method and |
|
This is great! I'm interested in taking this one step further after this PR is merged and flattening some of the wallet related abstractions now that there is just Askar. |
dbluhm
approved these changes
Nov 5, 2024
ff137
reviewed
Nov 6, 2024
Comment on lines
-115
to
-119
| async def test_ready_middleware_http_unauthorized(self): | ||
| """Test handling of web.HTTPUnauthorized and related exceptions.""" | ||
| with mock.patch.object(test_module, "LOGGER", mock.MagicMock()) as mock_logger: | ||
| mock_logger.info = mock.MagicMock() | ||
|
|
There was a problem hiding this comment.
Seems that the rebase dropped some of the unit tests I added :-) from #3327
I'll re-add in follow-up PR 👍
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.