google / gtest-parallel

Run Google Test suites in parallel.
Apache License 2.0
417 stars 104 forks source link

Add a mechanism for avoiding removing non-gtest-parallel data in output_dir #44

Closed pbos closed 7 years ago

pbos commented 7 years ago

Currently, if a user sets --output_dir=/path/to/important/data, gtest-parallel will clear that out from any files to remove files from old test runs.

Some different solutions would be: 1: Warn if the directory exists, but does not contain a .gtest-parallel-logs file (that we would start saving). This would be a bad upgrade path for existing directories, since it would require user intervention. 2: Log to gtest-parallel/ subdirectory, and remove logs from in here. This would change the --output_dir default to /tmp instead of /tmp/gtest-parallel, which would then be joined into /tmp/gtest-parallel in the end. This heuristic would only fail if a user does --output_dir=path/to/git-dir when it contains gtest-parallel, which I think we can ignore. 3: We could also call this subdirectory in 2 gtest-parallel-logs, which should never overlap. 4: Only remove *.log, but eew. 5: Refuse to log to any directories that contain non-*.log files. Also eew.

I don't like 1, because if you run it once to /path/to/important/data, select "ok", and then run it again, we nuke important data anyways. I think I like 2 or 3 best, not sure which. Both these solutions require that users know that we log to a subdirectory. Though given that we do passed/, failed/ and interrupted subdirectories already this might be OK, but it depends on if users are scripting against known subdirectories. @ehlemur, do you make use of how subdirectories under --output_dir are organized?

ehlemur-zz commented 7 years ago

We do passed/, failed/ and interrupted/ so that it's easier for the user to find the failures. I'd like to keep that. But doing that inside a gtest-parallel[-logs] sub-directory would be fine.

pbos commented 7 years ago

Sounds good, CL up for review. I think taking the hit of changing /tmp/gtest-parallel for our users to /tmp/gtest-parallel-logs is worth it to fix this foot-shooter.