Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Deleting all images that have passed tests before upload#27882
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Modified code to delete all types of files which have not failedMinor extension handlingChecking if file exists before deletingDid not delete directories
Hey, could someone help me out on how to resolve the failing tests? |
@Impaler343 those tests seem to be a bit flakey at the moment as they are failing across multiple PRs. I have re-spun them so we can see if they pass this time. |
.github/workflows/tests.yml Outdated
| if [[ -e "${base}.$extension" ]]; then | ||
| rm "${base}.$extension" | ||
| fi | ||
| echo "Removed $file" |
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.
| echo "Removed $file" | |
| echo "Removed ${base}.$extension" |
?
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.
Fixed that. Another thing - Should I delete all empty folders inside theresult_images folder? Or leave it as it is
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 think leaving empty directories is not something worth holding the PR over, but if it is easy no reason not to do it.
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 guess this works well as it is, don't want to break anything in the process
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 real test will be the next time we have broken test images and need to down load them...
Impaler343 commentedMar 21, 2024 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I guess this is ready to merge, do you expect any more changes from my side? |
@Impaler343 since@tacaswell already approved this one, you now need a second reviewer to also approve and then it can be merged (we require two approvals for most things except documentation changes). |
ping@QuLogic, as he had reviewed a similar PR. Please review :) |
Uh oh!
There was an error while loading.Please reload this page.
PR summary
Supersedes PR#27881 (I'm so sorry for creating unnecessary traffic on this repo.)
Attempts toclose#23400 and potentially complete what PR#19466 intended to do
Included a bash script right before the uploading part of the code in thetests.yml file to delete all files(.png, .svg, .eps, .pdf) that have not failed in theresult_images directory. Have tested this using a dummy bash file and the downloaded result_images folder. Please let me know if this approach seems correct, or is blatantly wrong :).
PR checklist