feat(besu-test-ledger): send funds to already created address #2251
Conversation
AndreAugusto11
requested review from
petermetz,
takeutak,
izuru0,
jagpreetsinghsasan and
sandeepnRES
as code owners
|
FYI @RafaelAPB @petermetz the |
|
@petermetz as @AndreAugusto11 said, the ubiquity connector tests fail, because this workspace does not have an api key for that connector. Tests only run successfully on my repo. Given this, shall we remove the ubiquity connector tests from the CI? |
There was a problem hiding this comment.
@AndreAugusto11 LGTM but please fix the test(s) that started failing:
@RafaelAPB I'd rather just "power through it" as-in, fix the failing tests as soon as possible instead of removing them. Seeing the tests fail on every PR (pointlessly) is annoying (to me at least) but this way it is a constant reminder that we should really fix them... :-) |
AndreAugusto11
force-pushed
the
feat-2250
branch
from
93f4efe
to
7586c6f
Compare
AndreAugusto11
force-pushed
the
feat-2250
branch
from
7586c6f
to
bafae3f
Compare
@petermetz I am able to run the failing tests related to Besu locally ( get-balance-endpoint.test.ts.log |
|
The tests are failing because we are not giving them a valid API key for Ubiquity. Is the solution you to generate one and put it on the env vars of the project? |
@RafaelAPB @AndreAugusto11 Sorry, I forgot about that. We need to fix it somehow but not in scope for this PR so nvm on that topic for here. |
@AndreAugusto11 If they pass locally then it's probably just a flake that it did not on the CI. I've done a rebase onto upstream main and force pushed it for you. Let's see how it goes now but I'll approve in the meantime because there's no other problem with it AFAICT. |
There was a problem hiding this comment.
@AndreAugusto11 LGTM, thank you and sorry for the slow reviews!
There was a problem hiding this comment.
@AndreAugusto11 @RafaelAPB The test is failing consistently now unfortunately. Please remedy and then it's good to go AFAICT!
Attaching the CI logs - fails the same way on my local machine so I'm pretty sure it's not a fluke.
2023-04-03T01-53-43_PR_2251_besu_test_crash.log
@petermetz thank you very much! This is weird because I have been trying to run those tests locally, and they pass. The changes are minimal, and I cannot understand where the problem might be. Do you have any idea? |
Enable sending funds to an existing account New method created: * sendEthToAccount Closes hyperledger-cacti#2250 Co-authored-by: Peter Somogyvari <[email protected]> Signed-off-by: André Augusto <[email protected]> Signed-off-by: Peter Somogyvari <[email protected]>
There was a problem hiding this comment.
@AndreAugusto11 You were missing an await which looks like ended up creating a race condition - at least that's what I'm thinking for now. LGTM
Enable sending funds to an existing account.
New method created:
closes #2250
Signed-off-by: André Augusto [email protected]