-
Notifications
You must be signed in to change notification settings - Fork 123
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
Make testEnv implement Autoclosable Interface, replace occurrences #2023
Make testEnv implement Autoclosable Interface, replace occurrences #2023
Conversation
need to take a look at the indentation changes yet, so i switched this PR to WIP |
1febc0b
to
df872b9
Compare
ready for review! |
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.
Hey, nice work!
DCO check is failing because the commits are not signed off. Please check out https://github.com/hashgraph/.github/blob/main/CONTRIBUTING.md and see how to set up your GPG keys.
When you've setup the keys you can rebase your commits while signing them.
https://github.com/hashgraph/hedera-sdk-java/pull/2023/checks?check_run_id=31080572033
df872b9
to
2ba597c
Compare
Signed-off-by: steffenboe <[email protected]> check tab indentation fix Signed-off-by: steffenboe <[email protected]> check tab indentation fix, 2nd try Signed-off-by: steffenboe <[email protected]> check tab indentation fix, 3rd try Signed-off-by: steffenboe <[email protected]> check tab indentation fix, 4th try Signed-off-by: steffenboe <[email protected]> Fix intendation Fix indentation
2ba597c
to
da01fad
Compare
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.
Looks good, there are some usages missing thought, like in TokenGrantKycIntegrationTest
and TransactionResponseTest
you're correct, and these tests aren't the only ones where a replacement is not possible. this is because the close()-method gets called with parameters there. this is not conforming to the Autoclosable interface, and so these occurrences can't be replaced with a |
@steffenboe You are right, I had forgotten about that! I was actually thinking a while back to rework the Would you like to rework the public void close() throws Exception {
if (!operatorId.equals(originalClient.getOperatorAccountId())) {
var hbarsBalance = new AccountBalanceQuery()
.setAccountId(operatorId)
.execute(originalClient)
.hbars;
new TransferTransaction()
.addHbarTransfer(operatorId, hbarsBalance.negated())
.addHbarTransfer(Objects.requireNonNull(originalClient.getOperatorAccountId()), hbarsBalance)
.freezeWith(originalClient)
.signWithOperator(client)
.execute(originalClient);
client.close();
}
originalClient.close();
} And then you could finish the rest of the tests to use |
Sounds good! Will work on that ;) |
Signed-off-by: steffenboe <[email protected]>
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.
LGTM, TY @steffenboe
…iero-ledger#2023) Signed-off-by: steffenboe <[email protected]> Co-authored-by: Ivan Ivanov <[email protected]> Signed-off-by: Jeffery Orazulike <[email protected]>
…iero-ledger#2023) Signed-off-by: steffenboe <[email protected]> Co-authored-by: Ivan Ivanov <[email protected]> Signed-off-by: Jeffery Orazulike <[email protected]>
…iero-ledger#2023) Signed-off-by: steffenboe <[email protected]> Co-authored-by: Ivan Ivanov <[email protected]>
Description:
Makes
IntegrationTestEnv
implement Autoclosable and replaces occurrences in tests withtry-with-resources
-block.Related issue(s):
Fixes #
#1975
Notes for reviewer:
Checklist