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

Fix state_machine teardown in tests #1941

Merged
merged 1 commit into from
Sep 10, 2024
Merged

Fix state_machine teardown in tests #1941

merged 1 commit into from
Sep 10, 2024

Conversation

AhmedSoliman
Copy link
Contributor

@AhmedSoliman AhmedSoliman commented Sep 10, 2024

Fix state_machine teardown in tests

Correctly shutdown rocksdb manager in those tests. This fixes the occasional segfaults in CI tests

Correctly shutdown rocksdb manager in those tests. This fixes the occasional segfaults in CI tests
Copy link
Contributor

@slinkydeveloper slinkydeveloper left a comment

Choose a reason for hiding this comment

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

I guess this works until you don't panic in the tests, right?

@@ -87,6 +87,7 @@ async fn send_with_delay() {
test_env.storage.get_invocation_status(&invocation_id).await,
ok(pat!(InvocationStatus::Invoked { .. }))
);
test_env.shutdown().await;
Copy link
Contributor

@slinkydeveloper slinkydeveloper Sep 10, 2024

Choose a reason for hiding this comment

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

ahhhh i miss so much junit with before/after steps :( I wish something similar would exists in rust.

Copy link

github-actions bot commented Sep 10, 2024

Test Results

15 files  ±0  15 suites  ±0   22m 7s ⏱️ +15s
 6 tests ±0   6 ✅ ±0  0 💤 ±0  0 ❌ ±0 
18 runs  ±0  18 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit b501c38. ± Comparison against base commit 36b90d7.

♻️ This comment has been updated with latest results.

@AhmedSoliman
Copy link
Contributor Author

I guess this works until you don't panic in the tests, right?

Yes. I'm not so concerned about the panic case since the entire process will stop.

@AhmedSoliman AhmedSoliman merged commit d8f9e49 into main Sep 10, 2024
19 checks passed
@AhmedSoliman AhmedSoliman deleted the pr1941 branch September 10, 2024 08:48
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.

2 participants