-
Notifications
You must be signed in to change notification settings - Fork 13
ci: compatibility and formatting fixes #91
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
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.
Nice cleanups, but I think you may have found a problem with the way fsim renames the file to the destination (see test logs). Funny how this doesn't happen on the client-side which IIUC is also renaming files to do the move.
Signed-off-by: Miguel Martín <[email protected]>
Signed-off-by: Miguel Martín <[email protected]>
Signed-off-by: Miguel Martín <[email protected]>
Use `./test/workdir` as the default `base_dir` instead of `/tmp/go-fdo` as the directory where the test files will be created. Configure the container's working dir through the `working_dir` variable (default: `/workdir`) Signed-off-by: Miguel Martín <[email protected]>
Create the temp files in the upload destination folder instead of `/tmp` to avoid "invalid cross-device link" errors. Signed-off-by: Miguel Martín <[email protected]>
That was fixed on client side by creating the temporary file in the final destination dir: fido-device-onboard/go-fdo-client@9ac6738 Pretty much the same I am proposing on this PR for the server side :) |
| uninstall_server | ||
| uninstall_client | ||
| rm -rf "${base_dir}" | ||
| rm -rf "${base_dir:?}"/* |
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.
Oh, yes this is indeed a good change 🥇
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.
LGTM: local testing successful, shipit.
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.
LGTM
No description provided.