search

stellar / stellar-cli

CLI for Stellar developers
70 stars 70 forks source link

Sleep is required to pass tests. See https://github.com/stellar/soroban-cli/pull/1202 #1231

Open willemneal opened 7 months ago

willemneal commented 7 months ago

What version are you using?

What did you do?

What did you expect to see?

What did you see instead?

Botdavid commented 3 weeks ago

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Am a software engineer

How I plan on tackling this issue

I will go through the documentation and implement the all the instruction..

KoxyG commented 3 weeks ago

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

My name is Koxy. I'm a blockchain Rust developer and Stellar ecosystem contributor. As an SCF Pathfinder and contributor to Stellar documentation, I have hands-on experience with both Rust and the Stellar platform.

My background in open-source development and familiarity with Stellar's architecture positions me well to address the timing issues in Soroban CLI tests.

I'm eager to implement immediate solutions while collaborating with the team on long-term improvements to enhance Soroban CLI's reliability.

How I plan on tackling this issue

To address the timing issues in the Soroban CLI tests, I would:

  1. Analyze: First, I'd thoroughly review the failing tests and surrounding code to understand the root causes of the timing issues.

  2. Implement Quick Fix: Add strategic sleep statements to stabilize the tests, as a temporary solution to unblock development.

  3. Document: Comment on the added sleep, explaining its purpose and marking it as temporary.

  4. Test Thoroughly: Ensure the modified tests pass consistently across multiple runs and environments.

  5. Propose Long-Term Solutions: Investigate more robust alternatives such as:

    • Custom wait mechanisms that poll for specific conditions
    • Refactoring to better handle asynchronous operations
    • Utilizing Rust's async/await features more effectively

  6. Collaborate: Discuss findings and proposed long-term solutions with the team to align on the best path forward.

  7. Implement & Iterate: Based on team feedback, implement agreed-upon long-term fixes, iterating as necessary.

Luluameh commented 3 weeks ago

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I have experience in JavaScript and TypeScript, particularly in testing and debugging CLI tools. I understand the intricacies of test environments and the importance of reliable test execution.

How I plan on tackling this issue

I’d investigate the current test setup to identify why a sleep is required for tests to pass. I’d analyze the test timing and dependencies, then implement a solution to ensure the tests run reliably without unnecessary delays, potentially optimizing the test flow.

KoxyG commented 2 weeks ago

Helllo @janewang @willemneal for this other issue, sleep required to pass tests I have been trying to work on the test, but i noticed that when ever i run cargo test ...... It outputs issues about my connection. "error: error trying to connect: tcp connect error: Connection refused".

Was i supposed to run a command to start up my connection in my local environment?.

willemneal commented 2 weeks ago

Yes you need to start a quickstart server.

If you are in the repo you can use cargo s which is an alias for cargo run --quiet --

cargo s network container start --ports-mapping 8889:8000 local
willemneal commented 2 weeks ago

Also this issues likely stems from the fact that multiple tests are trying to upload the same wasm binary at the same time.

A possible solution is to modify the Wasm binary for each test, by adding some random data to a custom section. This would mean that each test would have a unique wasm file to upload. The sleeps acted as a way for the tests no run sequentially which solved the issue of submitting the same wasm quickly.

Here is an example from loam, which had similar issues with its tests.

https://github.com/loambuild/loam/pull/88/files#diff-0e64c49f07fd665cae565b5863129f04cebefe45e23d9afd9125411cc4dbd08c

Checkout the rest of the PR for more details.

KoxyG commented 2 weeks ago

Also this issues likely stems from the fact that multiple tests are trying to upload the same wasm binary at the same time.

A possible solution is to modify the Wasm binary for each test, by adding some random data to a custom section. This would mean that each test would have a unique wasm file to upload. The sleeps acted as a way for the tests no run sequentially which solved the issue of submitting the same wasm quickly.

Here is an example from loam, which had similar issues with its tests.

https://github.com/loambuild/loam/pull/88/files#diff-0e64c49f07fd665cae565b5863129f04cebefe45e23d9afd9125411cc4dbd08c

Checkout the rest of the PR for more details.

okay, on this.

KoxyG commented 2 weeks ago

Hi @janewang @willemneal

I have fixed this issues. This test works fine now, sleep is no longer needed for test to pass. This is my pull request here. Kindly help review https://github.com/stellar/stellar-cli/pull/1651