-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Improve tsh tests #5394
Improve tsh tests #5394
Conversation
ee725f4
to
544c129
Compare
utils.InitLoggerForTests(testing.Verbose()) | ||
os.RemoveAll(client.FullProfilePath("")) | ||
|
||
defer os.RemoveAll(client.FullProfilePath("")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found the the removal of the default tsh login extremely hideous when working with the external environments (after a successful tsh login
) and with (unit) tests - I think we should make an effort with replacing this with a temporary location as is otherwise customary for testing. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, but currently tsh
behavior does not allow custom profile paths. It should, but, since the clearing behavior predates this PR, I'd prefer to add that functionality as a separate PR with a decent amount of manual testing to ensure no regressions.
544c129
to
b0b0a29
Compare
b0b0a29
to
08ff453
Compare
08ff453
to
46356e6
Compare
Various improvements to tests/testability for
tsh
, including coverage for #5323