Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33
GH-606: Initialize start_block to none to use latest block#374
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
ed7ca69c61be52aa54ff02a0e285c965035f7938de89e7171382f0de7c9b69da9c7fafFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,8 @@ | ||
| # Copyright (c) 2019, MASQ (https://masq.ai) and/or its affiliates. All rights reserved. | ||
| FROM trufflesuite/ganache-cli:v6.7.0 | ||
| FROM trufflesuite/ganache-cli:v6.12.2 | ||
| ADD ./entrypoint.sh /app/ | ||
| EXPOSE 18545 | ||
| ENTRYPOINT /app/entrypoint.sh | ||
| ENTRYPOINT ["/app/entrypoint.sh"] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,8 @@ | ||
| #!/bin/sh | ||
| node /app/ganache-core.docker.cli.js -p 18545 --networkId 2 --verbose --mnemonic "timber cage wide hawk phone shaft pattern movie army dizzy hen tackle lamp absent write kind term toddler sphere ripple idle dragon curious hold" | ||
| node /app/ganache-core.docker.cli.js \ | ||
| -h 0.0.0.0 \ | ||
| -p 18545 \ | ||
| --networkId 2 \ | ||
| --verbose \ | ||
| --mnemonic "timber cage wide hawk phone shaft pattern movie army dizzy hen tackle lamp absent write kind term toddler sphere ripple idle dragon curious hold" | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not consistent on how we should finish this branch regarding these files in the docker folder. Do you think it is not reasonable to put the master alike code in here? Especially the comment speaks about things that cannot be found on your branch, instead the GH-711 has it. Deleting the comment is necessary, therefore I'm suggesting to undo the changes in both files: ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Master does not have Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No probs. I'm no longer concerned about this. You can leave it in if you want to. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -241,8 +241,10 @@ impl MockBlockchainClientServer { | ||
| let mut requests = requests_arc.lock().unwrap(); | ||
| requests.push(body); | ||
| } | ||
| let response = responses.remove(0); | ||
| Self::send_body(conn_state, response); | ||
| if !responses.is_empty() { | ||
| let response = responses.remove(0); | ||
| Self::send_body(conn_state, response); | ||
| } | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you sure this is something we want to have? I don't think so. Let me check if I understand what is going on here: this is a mock server which is supposed to serve requests as if sent to a real blockchain service. And I do think each such a test should also contain the second part, where it awaits a response. The response needs to be set up in the mock before the actual test starts off, if this step is left out it is actually an error of the programmer and therefore it's adequate to halt the test if we happen to find out there's been a request but we can't reach in for a prepared response. That's actually this very moment which you bypassed with the extra if statement. Correct me if I can't see the improvement beyond your modification. However, unless you do, I'd definitely revert these lines. ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I only changed this because | ||
| let _ = notifier_tx.send(()); // receiver doesn't exist if test didn't set it up | ||
| } | ||
| None => (), | ||
| @@ -437,7 +439,7 @@ mod tests { | ||
| .response("Thank you and good night", 40) | ||
| .start(); | ||
| let mut client = connect(port); | ||
| client.write(b"POST /biddle HTTP/1.1\r\nContent-Length: 5\r\n\r\nfirstPOST /biddle HTTP/1.1\r\nContent-Length: 6\r\n\r\nsecond").unwrap(); | ||
| client.write(b"POST /biddle HTTP/1.1\r\nContent-Length: 5\r\n\r\nfirstPOST /biddle HTTP/1.1\r\nContent-Length: 6\r\n\r\nsecond").unwrap(); | ||
| let (_, body) = receive_response(&mut client); | ||
| assert_eq!( | ||
| @@ -567,7 +569,7 @@ mod tests { | ||
| assert_eq!(notified.try_recv().is_err(), true); | ||
| let requests = subject.requests(); | ||
| assert_eq!(requests, vec![ | ||
| assert_eq!(requests, vec![ | ||
| "POST /biddle HTTP/1.1\r\nContent-Type: application-json\r\nContent-Length: 82\r\n\r\n{\"jsonrpc\": \"2.0\", \"method\": \"first\", \"params\": [\"biddle\", \"de\", \"bee\"], \"id\": 40}".to_string(), | ||
| "POST /biddle HTTP/1.1\r\nContent-Type: application-json\r\nContent-Length: 48\r\n\r\n{\"jsonrpc\": \"2.0\", \"method\": \"second\", \"id\": 42}".to_string(), | ||
| "POST /biddle HTTP/1.1\r\nContent-Type: application-json\r\nContent-Length: 47\r\n\r\n{\"jsonrpc\": \"2.0\", \"method\": \"third\", \"id\": 42}".to_string(), | ||
| @@ -600,7 +602,7 @@ mod tests { | ||
| r#"{"jsonrpc": "2.0", "result": {"name":"Billy","age":15}, "id": 42}"# | ||
| ); | ||
| let requests = subject.requests(); | ||
| assert_eq!(requests, vec![ | ||
| assert_eq!(requests, vec![ | ||
| "POST / HTTP/1.1\r\ncontent-type: application/json\r\nuser-agent: web3.rs\r\nhost: 172.18.0.1:32768\r\ncontent-length: 308\r\n\r\n{\"jsonrpc\":\"2.0\",\"method\":\"eth_getLogs\",\"params\":[{\"address\":\"0x59882e4a8f5d24643d4dda422922a870f1b3e664\",\"fromBlock\":\"0x3e8\",\"toBlock\":\"latest\",\"topics\":[\"0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef\",null,\"0x00000000000000000000000027d9a2ac83b493f88ce9b4532edcf74e95b9788d\"]}],\"id\":0}".to_string() | ||
| ]) | ||
| } | ||
| @@ -704,6 +706,6 @@ mod tests { | ||
| body.len(), | ||
| body | ||
| ) | ||
| .into_bytes() | ||
| .into_bytes() | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -38,10 +38,7 @@ use web3::Web3; | ||
| #[test] | ||
| fn verify_bill_payment() { | ||
| let mut cluster = match MASQNodeCluster::start() { | ||
| Ok(cluster) => cluster, | ||
| Err(e) => panic!("{}", e), | ||
| }; | ||
| let mut cluster = MASQNodeCluster::start().unwrap(); | ||
| let blockchain_server = BlockchainServer { | ||
| name: "ganache-cli", | ||
| }; | ||
| @@ -64,7 +61,7 @@ fn verify_bill_payment() { | ||
| assert_balances( | ||
| &contract_owner_wallet, | ||
| &blockchain_interface, | ||
| "99998043204000000000", | ||
| "99998381140000000000", | ||
bertllll marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| "472000000000000000000000000", | ||
| ); | ||
| let payment_thresholds = PaymentThresholds { | ||
| @@ -189,7 +186,7 @@ fn verify_bill_payment() { | ||
| assert_balances( | ||
| &contract_owner_wallet, | ||
| &blockchain_interface, | ||
| "99998043204000000000", | ||
| "99998381140000000000", | ||
| "472000000000000000000000000", | ||
| ); | ||
| @@ -235,7 +232,7 @@ fn verify_bill_payment() { | ||
| assert_balances( | ||
| &contract_owner_wallet, | ||
| &blockchain_interface, | ||
| "99997886466000000000", | ||
| "99998223682000000000", | ||
| "471999999700000000000000000", | ||
| ); | ||
| @@ -330,7 +327,7 @@ fn assert_balances( | ||
| assert_eq!( | ||
| format!("{}", eth_balance), | ||
| String::from(expected_eth_balance), | ||
| "Actual EthBalance {} doesn't much with expected {}", | ||
| "Actual EthBalance {} doesn't match with expected {}", | ||
| eth_balance, | ||
| expected_eth_balance | ||
| ); | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is actually a bad test because if the function returns an err we want to know if it is an appropriate one, or its message meaningful.
I'd like to see two-sides assertion with Ok(()) or Err("The error message").
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this now does what you mean... 2 separate test methods one for Err cases and this one for Ok cases.