-
Notifications
You must be signed in to change notification settings - Fork 475
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
Added timeout for mount command in goss #877
Conversation
@aelsabbahy Could you check this ? |
OSS build credits ran out with Travis CI. I'll open a support ticket for them. Once the credits are available, I'll close and reopen this PR to trigger the build. |
CI is back up and running. Once the build is passed, I can review the code changes. |
@aelsabbahy The CI passed. Please have a look. |
Eyeball check, this looks good. I assume this PR works locally for you for your needs. It doesn't seem to break any of the CI tests which adds confidence. A few minor questions:
Many thanks for submitting this btw. Just trying to understand the problem better before merging this solution. |
Thanks @aelsabbahy for the review.
|
Once this branch is updated it looks good to me assuming it correctly solved your issue. I'm not sure how to reproduce it locally to test the failure situation. 😅 |
cb578d5
to
91dc353
Compare
Added goss timeout in integration tests Added timeout to goss integration tests added goss timeout added goss timeout added mount to goss added timeout to goss
f9e7c72
to
da769f0
Compare
@aelsabbahy please take a look and merge the pr if good. Thanks :) |
I'm on travel currently, I will review, merge, and release this in ~11 days or so. I'll make sure no other PRs are merged before this to avoid any more merges/rebases. Thank you for the contribution, it's nice to see niche problems at scale 😄 |
@aelsabbahy Could you please review this once ? |
Yup, I'll review it once I'm back from travel. I'm flying today, so sometime before the end of this weekend should be ample time for me to take a look at this. |
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.
Merging, thank you for the contribution! This is planned to be in the next goss release.
Checklist
Description of change