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

Technical Debt: Wallet Layer & Network 'listen' testing #115

Merged
merged 2 commits into from
Mar 25, 2019

Conversation

KtorZ
Copy link
Member

@KtorZ KtorZ commented Mar 22, 2019

Issue Number

#94

Overview

  • I have removed unused code from 'watchWallet' in the wallet layer
  • I have added an integration test which syncs a wallet for a few seconds using the 'listen' and 'watchWallet' functions available from the network and wallet layers.

Comments

@KtorZ KtorZ self-assigned this Mar 22, 2019
@KtorZ KtorZ requested a review from Anviking March 22, 2019 18:30
@KtorZ KtorZ force-pushed the master branch 2 times, most recently from 049fddd to a8d7c7d Compare March 23, 2019 12:45
threadDelay 5000000
cancel handle
tip <- currentTip <$> unsafeRunExceptT (getWallet wallet wid)
tip `shouldSatisfy` (> SlotId 0 0)
Copy link
Member

Choose a reason for hiding this comment

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

Nice!

@Anviking Anviking force-pushed the KtorZ/94/technical-debt-walletlayer-testing branch from 2458611 to e365b4b Compare March 25, 2019 14:22
@Anviking
Copy link
Member

I fixed missing import error, but tests are failing for me

  test/integration/Cardano/Wallet/Network/HttpBridgeSpec.hs:50:13:
  1) Cardano.Wallet.Network.HttpBridge, Happy paths, get from packed epochs
       expected: Right 21600
        but got: Left (NodeUnavailable "Connection error: HttpExceptionRequest Request {\n  host                 = \"localhost\"\n  port                 = 1337\n  secure               = False\n  requestHeaders       = [(\"Accept\",\"text/plain\")]\n  path                 = \"/testnet/tip\"\n  queryString          = \"\"\n  method               = \"GET\"\n  proxy                = Nothing\n  rawBody              = False\n  redirectCount        = 10\n  responseTimeout      = ResponseTimeoutDefault\n  requestVersion       = HTTP/1.1\n}\n (ConnectionFailure Network.Socket.connect: <socket: 15>: does not exist (Connection refused))")

Ah:

thread 'main' panicked at 'start http server: Io(Os { code: 48, kind: AddrInUse, message: "Address already in use" })', src/libcore/result.rs:1009:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.

Not sure if we should use different ports, or make sure they are not running at the same time.

@Anviking Anviking force-pushed the KtorZ/94/technical-debt-walletlayer-testing branch from e365b4b to 0caf1de Compare March 25, 2019 14:41
@@ -89,6 +90,7 @@ spec = do
newNetworkLayer =
HttpBridge.newNetworkLayer "testnet" port
startBridge = do
threadDelay 1000000
Copy link
Member

Choose a reason for hiding this comment

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

@KtorZ I added this to avoid Address already in use-problems. Should pass now, but not exactly sure what the problem was.

A failing log: https://travis-ci.org/input-output-hk/cardano-wallet/jobs/510990561

Copy link
Member Author

Choose a reason for hiding this comment

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

10 seconds is a rather long delay, and I wouldn't expect this to fail even without any delay between two successive runs of the spec.

Copy link
Member Author

Choose a reason for hiding this comment

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

🤔 ... having a look

Copy link
Member

Choose a reason for hiding this comment

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

I thought that was 1s 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

So yeah, it seems that we cancel from the async seems to wait for the delivery of the exception, but the socket isn't freed immediately by the OS. I've added a delay of 500ms which should be plenty of time already to free up the socket. I've re-run that 10x in a row without getting any issues 🤔

@KtorZ KtorZ force-pushed the KtorZ/94/technical-debt-walletlayer-testing branch from 0caf1de to 7472df7 Compare March 25, 2019 15:03
@KtorZ KtorZ force-pushed the KtorZ/94/technical-debt-walletlayer-testing branch from 7472df7 to 981fecd Compare March 25, 2019 15:31
@KtorZ KtorZ merged commit 063d9b4 into master Mar 25, 2019
@KtorZ KtorZ deleted the KtorZ/94/technical-debt-walletlayer-testing branch March 25, 2019 16:28
@KtorZ KtorZ mentioned this pull request Mar 26, 2019
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.

2 participants