-
Notifications
You must be signed in to change notification settings - Fork 2
dispatch run integration tests
#62
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
Conversation
d715f8a to
4d90bc8
Compare
4d90bc8 to
757e095
Compare
| // Check if the error is due to context timeout (command running too long) | ||
| if ctx.Err() == context.DeadlineExceeded { |
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.
The context error will always carry information about this being a timeout, could we simplify the code by removing this special case?
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.
It will do exit 1 on printenv ENV_VARIABLE, so I've added this check to be sure that this command exited properly and we can read stderr now to make sure ENV_VARIABLE has correct value (when err is nil).
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'm not sure I understand the condition in which we would get a timeout, but if you know it's important here it's fine (maybe worth a comment to record that context).
This PR adds initial integration tests for the
dispatch runcommand