Fixed the test file in dotenv.rs #1651
Conversation
leighmcculloch
left a comment
leighmcculloch
left a comment
There was a problem hiding this comment.
Hi @KoxyG! Thanks for contributing.
Can you make the following changes to the PR so we can merge?
-
Remove the refactors and minimise the change to the minimal changes required to fix the problem. Most of this PR seems to be a refactor, moving code around and not changing the functionality, which is unnecessary.
-
Remove the four dependencies added that are not used.
-
Re-add the test that got removed. We need to fix the problem without removing the test.
-
Undo the change to the contract address.
|
@leighmcculloch This issue was solved in the Loam repo by implementing what you suggested; adding some random bits to a Wasm binary so that each test has a unique contract. See https://github.com/loambuild/loam/pull/88/files#diff-0e64c49f07fd665cae565b5863129f04cebefe45e23d9afd9125411cc4dbd08cR74-R99 |
|
Hi @leighmcculloch i have made the neccessary changes. Kindly help review |
|
we will be closing this out if there is no activity for a week |
It's still under review. I haven't been updated about the progress of this by the reviewers. |
leighmcculloch
left a comment
leighmcculloch
left a comment
There was a problem hiding this comment.
Lgtm if the sleeps are no longer needed. I've approved the builds to run to see.
One question inline below.
| let result = e | ||
| .new_assert_cmd("contract") | ||
| .env( | ||
| "SOROBAN_CONTRACT_ID", | ||
| "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAFCT4", | ||
| "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAFCT4", | ||
| ) | ||
| .arg("invoke") | ||
| .arg("--id") | ||
| .arg(id) | ||
| .arg("--") | ||
| .arg("hello") | ||
| .arg("--world=world") | ||
| .assert() | ||
| .stdout("[\"Hello\",\"world\"]\n"); | ||
| .assert(); | ||
|
|
||
| result.stdout("[\"Hello\",\"world\"]\n").success(); |
There was a problem hiding this comment.
Why do these lines need to change? I see that a .success() has been added, and the stdout has been moved to after the .assert(). What's the significance of those changes?
6 participants
What
Test files no longer needs sleep is no longer needed for test cases to pass.