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

SUP-1372 fix compressed #91

Merged
merged 14 commits into from
Sep 14, 2023
Merged

SUP-1372 fix compressed #91

merged 14 commits into from
Sep 14, 2023

Conversation

HugeIRL
Copy link
Contributor

@HugeIRL HugeIRL commented Sep 5, 2023

This PR adds a check to the compressed: setting to make sure the file or directory exists before attempting to compress. It will also not attempt to compress if the file/folder is either missing or empty, but display an error message about being unable to compress, and then continue on running instead of outright failing.

A new test has been added to check for path existence on compressed: for the expected graceful error and continue message.

A new test has been added to check for path existence on compressed when ignore-missing is set for the expected graceful error and continue message.

The rest of the affected failing tests have also been updated to create each tests mock file and delete it after the test is done so the tests no longer fail due to the new existence check.

This will fix #89

Copy link
Contributor

@pzeballos pzeballos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the tests, you don't need to create a file *.log (and remove it later). That is actually an issue with the plugin that glob paths are not supported with compressed. So in this case because you are changing the logic, these tests started to fail. Instead of creating that file, you can simply update the variable of the test to use a particular file (like we have in the docs), for example:
export BUILDKITE_PLUGIN_ARTIFACTS_UPLOAD="file.log"

We also need to add a test when the ignore-missing is set and the file doesn't exist

Copy link
Contributor

@pzeballos pzeballos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! I added those two suggestions and did a small change in the docs 🥳

@pzeballos pzeballos merged commit 030a835 into master Sep 14, 2023
@pzeballos pzeballos deleted the SUP-1372-fix-compressed branch September 14, 2023 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ingore-missing doesn't work with compressed
2 participants