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

RSDK-8217 Add client logs to CLI tests #4707

Merged
merged 2 commits into from
Jan 14, 2025

Conversation

benjirewis
Copy link
Member

RSDK-8217

Allows passing in loggers to CLI actions that establish connections to machines with client.New. Passes in test loggers to those actions during testing.

This will not necessarily ensure we're outputting debug client logs from future tests that end up calling client.New, but, from what I can tell, that's not super trivial to ensure, and my guess is this ticket was mostly to make sure we can see debug logs from the shell service tests.

@benjirewis benjirewis requested a review from cheukt January 13, 2025 20:09
@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Jan 13, 2025
@benjirewis benjirewis force-pushed the cli-test-client-logs branch from 57013ec to 9f32cd9 Compare January 13, 2025 22:27
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jan 13, 2025
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jan 14, 2025
@benjirewis benjirewis merged commit e936b2c into viamrobotics:main Jan 14, 2025
16 checks passed
@benjirewis benjirewis deleted the cli-test-client-logs branch January 14, 2025 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test This pull request is marked safe to test from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants