fix(test): clean big files on clean recipe

[qlrd]

Description and Notes

This commit add a bash command to clean recipe and try to improve the clean process through some big temporary files. In some local tests, those jumped from 0 to 51G in /tmp/folder

How to verify the changes you have done?

See #848

[csgui]

Just some NITs about scripts portability. There are constrained environments that doesn't have bash by default.

[qlrd]

Just some NITs about scripts portability. There are constrained environments that doesn't have bash by default.

Applied some POSIX procedures on tests/run.sh and justfile.

[csgui]

Cool! It seems reasonable that scripts should be POSIX compatible unless there is a specific need that force them to use some other shell.

[csgui]

ACK 31b8cdb

[qlrd]

Cool! It seems reasonable that scripts should be POSIX compatible unless there is a specific need that force them to use some other shell.

yeah, CI will be unless we define in a job sh: bash. But it's a nice idea of yours to be "sh compatible"

[moisesPompilio]

I don't think these modifications are safe: contrib/clean_data.sh is intended to clean project tmp files (./tmp), but your change removes items from the system /tmp, which can interfere with other applications.

[moisesPompilio]

Why use the --preserve-data-dir flag?

[qlrd]

because if not set, they will delete logs and the logs job in CI will show an error

[moisesPompilio]

That doesn't make sense, because run.sh deletes the log before running the tests

[moisesPompilio]

humm, I think this won't work if FLORESTA_TEMP_DIR is set.

[jaoleal]

It wont, FLORESTA_TEMP_DIR is being ignored in clean_data.sh. If one needs to set FLORESTA_TEMP_DIR to another place, the script will fail to find it.

[jaoleal]

Approach NACK

The presented changes introduce unnecessary logic to filter out big files, it would be easier to just delete everything thats test-generated if the objective is to have clean environments to re run tests.

In #306, when clean_data.sh was introduced, we weren't creating any test directory for the integration tests, when they were introduced the cleaning logic was already thought, test data are placed under /tmp which is ephemeral.

[qlrd]

Approach NACK

The presented changes introduce unnecessary logic to filter out big files, it would be easier to just delete everything thats test-generated if the objective is to have clean environments to re run tests.

In #306, when clean_data.sh was introduced, we weren't creating any test directory for the integration tests, when they were introduced the cleaning logic was already thought, test data are placed under /tmp which is ephemeral.

unnecessary is to need manually remove every time i do a test in local machine with 51 GB in my /tmp folder when it reach a quota.

[qlrd]

I don't think these modifications are safe: contrib/clean_data.sh is intended to clean project tmp files (./tmp), but your change removes items from the system /tmp, which can interfere with other applications.

Are you run locally or just push to CI? I bet the second because if you do the first (and if you did the first IDK why it was ignored), every time you do it, you could see AFAIK 51GB on whatever could be FLORESTA_TEMP_DIR, either in ./tmp' or /tmp´. So if your concern is to affect the system, it's already affects who run tests locally.

[moisesPompilio]

NACK

I think the proposed changes don't add improvement to the project; changing clean_data.sh to another functionality doesn't make sense. Also, I ran the integration tests several times and the space they occupy is less than 400 MB, so I don't see the point in cleaning it; besides, it stays in /tmp by default which is cleaned automatically by the system.

[csgui]

PR closed since it is not actually need. Reopen discussion if need.