Skip to content
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

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

Closed
willemneal opened this issue Mar 1, 2024 · 9 comments
Assignees
Labels
bug Something isn't working stale

Comments

@willemneal
Copy link
Member

What version are you using?

What did you do?

What did you expect to see?

What did you see instead?

@willemneal willemneal added the bug Something isn't working label Mar 1, 2024
@Botdavid
Copy link

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
Copy link

KoxyG commented Sep 26, 2024

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
Copy link

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
Copy link

KoxyG commented Oct 3, 2024

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
Copy link
Member Author

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
Copy link
Member Author

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
Copy link

KoxyG commented Oct 3, 2024

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
Copy link

KoxyG commented Oct 4, 2024

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 #1651

Copy link
Contributor

github-actions bot commented Dec 7, 2024

This issue is stale because it has been assigned for 30 days with no activity. It will be closed in 30 days unless the stale label is removed, and the assignee is removed or updated.

@github-actions github-actions bot added the stale label Dec 7, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working stale
Projects
None yet
Development

No branches or pull requests

5 participants