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

T2. add(test): add test API that checks process logs for failures #3899

Merged
merged 16 commits into from
Mar 22, 2022

Conversation

teor2345
Copy link
Contributor

@teor2345 teor2345 commented Mar 17, 2022

Motivation

We want to check lightwalletd logs and Zebra logs for failures like:

  • missing RPCs
  • RPC argument parse errors
  • RPC argument data errors
  • RPC response parse errors
  • RPC response data errors
  • other specific errors logged by lightwalletd

But our testing framework makes it very hard to do that.

This PR adds an API to check test logs for failures, but it doesn't actually do those checks yet.

This is related to:

Designs

Add a set of failure regexes to each child process, and check each log line for those regexes. Panic if any failures are logged.

Make sure test processes are cleaned up correctly, so their logs are read and checked for failures.

Solution

Failure logs:

  • Add a set of failure regexes to test child processes, and use them to check process output
  • When the child is dropped, check any remaining output
  • Add internal tests for failure regex panics

Reading process output:

  • Add test APIs for consuming child output lines, without checking them for a success regex

Also document some edge cases, and some things we might want to fix later.

Review

This PR is not urgent.

It is based on PR #3892.

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behaviour
  • Tests for Errors

Follow Up Work

  • Check lightwalletd logs for failures

@teor2345 teor2345 added C-enhancement Category: This is an improvement P-Medium ⚡ C-testing Category: These are tests lightwalletd any work associated with lightwalletd labels Mar 17, 2022
@teor2345 teor2345 requested a review from a team as a code owner March 17, 2022 23:18
@teor2345 teor2345 self-assigned this Mar 17, 2022
@teor2345 teor2345 requested review from oxarbitrage and removed request for a team March 17, 2022 23:18
@teor2345 teor2345 force-pushed the lightwalletd-test-log-errors branch from 2af3008 to b3df126 Compare March 18, 2022 06:38
@teor2345 teor2345 force-pushed the test-log-failure-regex branch from 3c16b09 to 779a8f5 Compare March 18, 2022 06:41
@teor2345 teor2345 force-pushed the test-log-failure-regex branch from 7cc555f to 3e2cddf Compare March 18, 2022 09:09
@oxarbitrage oxarbitrage force-pushed the lightwalletd-test-log-errors branch from 3f6b1ff to 2a7c108 Compare March 20, 2022 15:52
@teor2345 teor2345 force-pushed the lightwalletd-test-log-errors branch from 2a7c108 to 3b7887a Compare March 20, 2022 21:37
@teor2345 teor2345 force-pushed the test-log-failure-regex branch from 3e2cddf to fcfc61e Compare March 20, 2022 21:40
Base automatically changed from lightwalletd-test-log-errors to main March 22, 2022 15:58
@oxarbitrage
Copy link
Contributor

Please rebase and fix conflicts when you get the time. Thanks!

@teor2345
Copy link
Contributor Author

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Mar 22, 2022

update

❌ Base branch update has failed

merge conflict between base and head
err-code: 94415

@teor2345
Copy link
Contributor Author

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Mar 22, 2022

rebase

❌ Base branch update has failed

Git reported the following error:

Rebasing (1/23)
Auto-merging zebra-test/src/command.rs
CONFLICT (content): Merge conflict in zebra-test/src/command.rs
Auto-merging zebra-test/src/command/to_regex.rs
CONFLICT (add/add): Merge conflict in zebra-test/src/command/to_regex.rs
error: could not apply 5ce0dee5... Make command test matching code accept generic regexes
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 5ce0dee5... Make command test matching code accept generic regexes

err-code: A5E44

@teor2345 teor2345 force-pushed the test-log-failure-regex branch from fcfc61e to 543db25 Compare March 22, 2022 20:37
@teor2345
Copy link
Contributor Author

Please rebase and fix conflicts when you get the time. Thanks!

Done!

The conflicts aren't real, they're just caused by squashing PRs.

Copy link
Contributor

@oxarbitrage oxarbitrage left a comment

Choose a reason for hiding this comment

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

Looks great, very complete with a lot of testing so it should work pretty much as we expect.

mergify bot added a commit that referenced this pull request Mar 22, 2022
@mergify mergify bot merged commit a5d7b9c into main Mar 22, 2022
@mergify mergify bot deleted the test-log-failure-regex branch March 22, 2022 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: This is an improvement C-testing Category: These are tests lightwalletd any work associated with lightwalletd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants