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

Decrease test_stdout's expected size: 400 -> 200 KiB #97

Merged
merged 1 commit into from
Oct 10, 2023
Merged

Decrease test_stdout's expected size: 400 -> 200 KiB #97

merged 1 commit into from
Oct 10, 2023

Conversation

tomfitzhenry
Copy link
Contributor

@tomfitzhenry tomfitzhenry commented Oct 10, 2023

Motivation: Make cargo test --release pass. This is useful for NixOS packaging, because NixOS tests against a release binary.

Currently the 'test_stdout' fails with --release, since the test expects the PDF to be > 400KiB, but the release PDF is 337 KiB.

Here we see the debug binary outputting 516 KB:

$ echo STDOUT | PAPERAGE_PASSPHRASE=secret cargo run  -- -o - | wc --bytes
    Finished dev [unoptimized + debuginfo] target(s) in 0.27s
     Running `target/debug/paper-age -o -`
515997

Here we see the release binary outputting 337KB:

$ echo STDOUT | PAPERAGE_PASSPHRASE=secret cargo run --release  -- -o - | wc --bytes
    Finished release [optimized] target(s) in 0.41s
     Running `target/release/paper-age -o -`
336980

So let's decrease the test limit. I guess the value of the test is mostly in checking that the PDF isn't less than < 10KB. The PDFs are visually indistinguishable from my eye.

Motivation: Make `cargo test --release` pass. This is useful for NixOS packaging, because NixOS tests against a release binary.

Currently the 'test_stdout' fails with `--release`, since the test expects the PDF to be > 400KiB, but the release PDF is 337 KiB.

Here we see the debug binary outputting 516 KB:

```
$ echo STDOUT | PAPERAGE_PASSPHRASE=secret cargo run  -- -o - | wc --bytes
    Finished dev [unoptimized + debuginfo] target(s) in 0.27s
     Running `target/debug/paper-age -o -`
515997
```

Here we see the release binary outputting 337KB:

```
$ echo STDOUT | PAPERAGE_PASSPHRASE=secret cargo run --release  -- -o - | wc --bytes
    Finished release [optimized] target(s) in 0.41s
     Running `target/release/paper-age -o -`
336980
```

So let's decrease the test limit. I guess the value of the test is mostly in checking that the PDF isn't less than < 10KB.
@codecov
Copy link

codecov bot commented Oct 10, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (11ce28c) 95.71% compared to head (fe5f165) 95.71%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #97   +/-   ##
=======================================
  Coverage   95.71%   95.71%           
=======================================
  Files           7        7           
  Lines         677      677           
=======================================
  Hits          648      648           
  Misses         29       29           
Files Coverage Δ
tests/cli.rs 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions
Copy link

PaperAge visual snapshots

Compare the output of the latest release version of PaperAge with the results from the current commit.

Latest release Current commit Diff
A4 A4 release A4 current A4 diff
Letter Letter release Letter current Letter diff

Encryption passphrase: supersecret

Note: Snapshots are deleted after 30 days.

@matiaskorhonen
Copy link
Owner

Thanks. Yeah, the test is there just a sanity check to ensure that the output is at least plausible…

@matiaskorhonen matiaskorhonen merged commit bd9797f into matiaskorhonen:main Oct 10, 2023
6 checks passed
@matiaskorhonen
Copy link
Owner

@tomfitzhenry FYI, I just released v1.1.4 which includes your changes.

@tomfitzhenry
Copy link
Contributor Author

@tomfitzhenry FYI, I just released v1.1.4 which includes your changes.

Great, thanks for that! :)

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.

2 participants