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

Wait for rbuilder running before running integration test #160

Merged
merged 1 commit into from
Aug 28, 2024

Conversation

ferranbt
Copy link
Contributor

📝 Summary

The new integration test introduced in #143 requires the rbuilder to be running to work. However, the playground setup was not waiting for it. This PR fixes that. It adds a loop after the rbuilder creation that checks in the output log for a specific log "RPC server job: started" that signals that the builder is running and the test can start.

💡 Motivation and Context


✅ I have completed the following steps:

  • Run make lint
  • Run make test
  • Added tests (if applicable)

Copy link

Benchmark results for b426ced

Report: https://flashbots-rbuilder-ci-stats.s3.us-east-2.amazonaws.com/benchmark/b426ced-7cb7f31/report/index.html

Date (UTC) 2024-08-28T13:53:28+00:00
Commit b426cedd0bb8d46ca5fce144cea503d82ad2044e
Base SHA 7cb7f3154d2fddcc51c87f78eb4b91c520ac7f8c

Significant changes

None

Copy link
Contributor

@metachris metachris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thxxx

Comment on lines +100 to +117
loop {
if start.elapsed().as_secs() > 10 {
return Err(PlaygroundError::Timeout);
}

// from the log file, check if the server has started
// by checking if the string "RPC server job: started" is present
let mut file = File::open(&log_path).map_err(|_| PlaygroundError::SetupError)?;
let mut contents = String::new();
file.read_to_string(&mut contents)
.map_err(|_| PlaygroundError::SetupError)?;

if contents.contains("RPC server job: started") {
break;
}

thread::sleep(std::time::Duration::from_millis(100));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this would be more idiomatic and avoid spamming io ops https://github.com/jmagnuson/linemux. I wouldn't call it a "must have" though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was hoping to do an http request to check some health check endpoint. But could not make it work.

@ferranbt ferranbt merged commit 7f7a5ae into develop Aug 28, 2024
3 checks passed
@ferranbt ferranbt deleted the ferranbt/wait-for-rbuilder-in-integration branch August 28, 2024 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants