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

Enforce zebra_test::command timeouts regardless of command output #1140

Closed
1 of 7 tasks
teor2345 opened this issue Oct 9, 2020 · 2 comments
Closed
1 of 7 tasks

Enforce zebra_test::command timeouts regardless of command output #1140

teor2345 opened this issue Oct 9, 2020 · 2 comments
Labels
A-rust Area: Updates to Rust code C-bug Category: This is a bug I-integration-fail Continuous integration fails, including build and test failures

Comments

@teor2345
Copy link
Contributor

teor2345 commented Oct 9, 2020

Version

zebra-test 3.0.0-alpha.0 (current main branch, commit eaf5473)

Platform

Linux 5.4.68 #1-NixOS SMP x86_64 GNU/Linux

Description

The zebra_test::command acceptance test framework contains a few undocumented surprises. The timeout is only checked in expect_stdout, and only after each newline. And the timeout kill check is inverted. (It appears that it has never been tested.)

Here's what I think we should fix:

  • Check if the command is running before killing it (Test sync on testnet #1053)
  • Enforce acceptance test command timeouts, even if the command doesn't output any lines
    • See the ignored tests in zebra-test/tests/command.rs
  • Enforce timeouts during wait_with_output
  • Limit the length of the output printed in the context
    • Allow developers to override the default via an env var, so they can see more output if needed
    • Print the first few and last few lines of output?
@teor2345 teor2345 added C-bug Category: This is a bug S-needs-triage Status: A bug report needs triage labels Oct 9, 2020
@teor2345 teor2345 modified the milestone: First Alpha Release Oct 9, 2020
teor2345 added a commit to teor2345/zebra that referenced this issue Oct 9, 2020
And add tests for the command functionality.

Also document some remaining bugs (see ZcashFoundation#1140).
@teor2345 teor2345 mentioned this issue Oct 9, 2020
3 tasks
teor2345 added a commit to teor2345/zebra that referenced this issue Oct 11, 2020
Some of these tests are ignored, because they trigger known TestDirExt
bugs. See ZcashFoundation#1140 for details.
dconnolly pushed a commit that referenced this issue Oct 15, 2020
And add tests for the command functionality.

Also document some remaining bugs (see #1140).
dconnolly pushed a commit that referenced this issue Oct 15, 2020
Some of these tests are ignored, because they trigger known TestDirExt
bugs. See #1140 for details.
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Feb 2, 2021
@teor2345
Copy link
Contributor Author

If we had this fix, we could probably have diagnosed the hang in #1773:

running 2 tests
test sync_large_checkpoints_mainnet ... ok
test sync_large_checkpoints_testnet ... test sync_large_checkpoints_testnet has been running for over 60 seconds
Error: The operation was canceled.

(after 1 hour)
https://github.com/ZcashFoundation/zebra/pull/1773/checks?check_run_id=1930291039#step:9:414

@teor2345
Copy link
Contributor Author

This hasn't been much of a problem recently, we fixed all the hangs.

@mpguerra mpguerra closed this as not planned Won't fix, can't repro, duplicate, stale May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rust Area: Updates to Rust code C-bug Category: This is a bug I-integration-fail Continuous integration fails, including build and test failures
Projects
None yet
Development

No branches or pull requests

2 participants