zstd icon indicating copy to clipboard operation
zstd copied to clipboard

Refactor progress bar & summary line logic

Open terrelln opened this issue 3 years ago • 4 comments

  • Centralize the logic about whether to print the progress bar or not in the *_PROGRESS() macros.
  • Centralize the logc about whether to print the summary line or not in FIO_shouldDisplayFileSummary() and FIO_shouldDisplayMultipleFileSummary().
  • Make --progress work for non-zstd (de)compressors.
  • Clean up several edge cases in compression and decompression progress printing along the way. E.g. wrong log level, or missing summary line.

One thing I don't like about stdout mode, which sets the display level to 1, is that warnings aren't displayed. After this PR, we could change stdout mode from lowering the display level, to defaulting to implied --no-progress. But, I think that deserves a separate PR.

terrelln avatar Jan 07 '22 23:01 terrelln

That's much clearer ! Thanks for the improvement !

One thing I don't like about stdout mode, which sets the display level to 1, is that warnings aren't displayed.

This corresponds to a zstdcat scenario : we want to avoid a situation where the output, presumed the console, intermix content and warning messages.

Cyan4973 avatar Jan 07 '22 23:01 Cyan4973

Btw, do we have tests that check the presence or absence of notification / progress / warning messages ? This could prove helpful, to ensure that some display properties are preserved across future modifications.

Cyan4973 avatar Jan 07 '22 23:01 Cyan4973

Btw, do we have tests that check the presence or absence of notification / progress / warning messages ? This could prove helpful, to ensure that some display properties are preserved across future modifications.

I agree, there are a lot of test cases that I ran manually that I would like to include in our test suite. playTests.sh isn't really set up to test the stderr output matches some expected output (with wildcards / regexes) in a sane way. Because there are tens of scenarios we'd want to test.

I'd rather have some way to tell the test suite to run a command, and check that the stderr output matches some file. There are definitely pre-built tools that can do this. So I'm going to spend a little bit of time trying to set it up.

terrelln avatar Jan 08 '22 00:01 terrelln

I'm writing a small tool to test running the CLI & checking stderr/stdout/exit code match some expectations.

I will submit the tool as a PR, then rebase this on top of it with the test cases.

terrelln avatar Jan 10 '22 17:01 terrelln