Added template support for Prawn PDF #57

kbrock · 2020-06-04T13:20:16Z

Adds #33 by @agios which uses a more recent prawn.

Sure, this version of prawn is still very old, and the gem does not look maintained, but this PR focuses on removing warnings that show up with recent ruby implementations.

This branch of the PR removes the conflict and updates the version of prawn and adds the table gem. But it is largely not my work. So thank you Agios.

The pdf test has been disabled because it seems to fail on every upgrade.
Will look into the issue behind this. The most recent failure is due to significant decimal places for 0 and some font tweaks.

The previous sample pdf had a table with headers but no data. Since this causes problems with later prawn versions, I added a data row.

coveralls · 2020-06-04T13:22:20Z

Pull Request Test Coverage Report for Build 82

17 of 56 (30.36%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-3.3%) to 93.339%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
lib/ruport/formatter/prawn_pdf.rb	17	56	30.36%

Totals
Change from base Build 78:	-3.3%
Covered Lines:	1051
Relevant Lines:	1126

💛 - Coveralls

kbrock · 2020-06-05T02:39:41Z

@jrafanie this is generating a bunch of warnings in ruby 2.6 (let alone 2.7)
So I wanted to merge this PR (and then run a sweep to clean up the remaining warnings and release / remove our custom gem)

Do you have any comments or concerns for this?
It probably won't concern us too much since this doesn't touch the models.

jrafanie · 2020-06-05T17:45:30Z

So I wanted to merge this PR (and then run a sweep to clean up the remaining warnings and release / remove our custom gem)

What's in our custom gem? I don't remember what feature we fixed or added.

Do you have any comments or concerns for this?

As long as it works for 2.5 since that's still supported, it should be fine.

kbrock · 2020-06-06T00:17:25Z

What's in our custom gem? I don't remember what feature we fixed or added.
-- @jrafanie

Modify a hash in place and freeze strings to reduce allocations. ManageIQ/ruport#1 - here: Modify a hash in place and freeze strings to reduce allocations. #34
fastercsv - done
fixing some tests for ruby 1.9 - done

As long as it works for 2.5 since that's still supported, it should be fine.

travis runs 2.5 - 2.7 (and this probably runs on 2.3, 2.4)

kbrock · 2020-06-06T00:21:09Z

kicking. trying to get new coverage numberss

not sure how stable these tests are pdfs that look the same can often be different in the binary

kbrock · 2020-06-06T00:37:55Z

The decrease in coverage is right.
A lot of code was added to prawn_pdf but tests were not added.
I wonder if there is a way to make the file test more stable.

probably best way to test this stuff is to add some mocking / stubs

kbrock · 2020-06-10T21:49:02Z

@jrafanie I looked over this code and got it working. you ok with these changes?

For ruby 2.7, upgrading prawn gets rid of a lot of warnings. Apps (including miq) end up with a ton of warnings without these changes.

The feature request for templates seem good, though I will probably never use them

jrafanie · 2020-06-11T14:05:57Z

test/prawn_pdf_formatter_test.rb


-    expected_output = IO.read(File.join(__dir__, 'expected_outputs/prawn_pdf_formatter/pdf_basic.pdf.test')).bytes
+  def xtest_render_pdf_basic


I'd prefer a comment as to why it's disabled here. Someone might assume this was a typo and try to fix it not knowing it's intentionally disabled. Maybe use the skip method in minitest?

https://www.paperlesspost.com/blog/teams/skipping-tests-minitest/

jrafanie

My only concern is the skipped tests without saying why. If they're flaky or inconsistent, we should make that obvious. The other changes look ok although some of the expected outputs test data changes look 😕 to me. I'm not sure what's going on to make them change.

kbrock · 2020-06-15T04:07:07Z

I'm not sure what's going on to make [the tests] change.
--@jrafanie

I changed the pdf we generated to not have an empty table. Certain versions of prawn were not handling that well. To be honest, I don't think an empty data table is that valid of a use case anyway.
Every time we change the version of prawn, it changes the pdf output. E.g.: it changed the number of decimal places when specifying x and y.

The 1 skipped test is essentially doing a diff of the output from prawn.
Any time the version of that library changes (which is probably not going to happen), then minor formatting changes make our tests fail.

I'll put this test back in, but it is fragile.

it is very unstable, so not a good test.

kbrock · 2020-06-17T18:10:32Z

reenabled the test

- Markdown formatter ruport#54 - Ruby 3.0 support - Reduced allocations to improve performance ruport#34 - Update Prawn version to 2.4.0 ruport#44 ruport#57 ruport#64

kbrock added the cleanup label Jun 4, 2020

kbrock force-pushed the prawn_pdf branch 2 times, most recently from 8ee0253 to e8a4c4c Compare June 5, 2020 00:31

kbrock changed the title ~~Prawn pdf~~ Added template support for Prawn pdf Jun 6, 2020

kbrock changed the title ~~Added template support for Prawn pdf~~ Added template support for Prawn PDF Jun 6, 2020

kbrock closed this Jun 6, 2020

kbrock reopened this Jun 6, 2020

agios and others added 4 commits June 5, 2020 20:22

Allow passing parameters to prawn table

457083f

Template support for PrawnPDF

dd3430a

Update prawn

735b54d

fix prawn table requirements

f75578c

not sure how stable these tests are pdfs that look the same can often be different in the binary

kbrock force-pushed the prawn_pdf branch from e8a4c4c to 02715fe Compare June 6, 2020 00:22

kbrock requested a review from jrafanie June 10, 2020 21:46

jrafanie reviewed Jun 11, 2020

View reviewed changes

jrafanie requested changes Jun 11, 2020

View reviewed changes

upgrade prawn (and disable the table test)

a668bcc

it is very unstable, so not a good test.

kbrock force-pushed the prawn_pdf branch from 02715fe to a668bcc Compare June 17, 2020 18:09

kbrock merged commit c7d8452 into ruport:master Jun 18, 2020

kbrock deleted the prawn_pdf branch June 18, 2020 21:47

kbrock added a commit to kbrock/ruport that referenced this pull request Sep 19, 2022

1.8.0 - 2022-09-20

0aa4bc5

- Markdown formatter ruport#54 - Ruby 3.0 support - Reduced allocations to improve performance ruport#34 - Update Prawn version to 2.4.0 ruport#44 ruport#57 ruport#64

kbrock mentioned this pull request Sep 19, 2022

1.8.0 - 2022-09-20 #65

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added template support for Prawn PDF #57

Added template support for Prawn PDF #57

kbrock commented Jun 4, 2020 •

edited

coveralls commented Jun 4, 2020 •

edited

kbrock commented Jun 5, 2020 •

edited

jrafanie commented Jun 5, 2020

kbrock commented Jun 6, 2020 •

edited

kbrock commented Jun 6, 2020 •

edited

kbrock commented Jun 6, 2020

kbrock commented Jun 10, 2020 •

edited

jrafanie Jun 11, 2020

jrafanie left a comment

kbrock commented Jun 15, 2020 •

edited

kbrock commented Jun 17, 2020


		expected_output = IO.read(File.join(__dir__, 'expected_outputs/prawn_pdf_formatter/pdf_basic.pdf.test')).bytes
		def xtest_render_pdf_basic

Added template support for Prawn PDF #57

Added template support for Prawn PDF #57

Conversation

kbrock commented Jun 4, 2020 • edited

coveralls commented Jun 4, 2020 • edited

Pull Request Test Coverage Report for Build 82

💛 - Coveralls

kbrock commented Jun 5, 2020 • edited

jrafanie commented Jun 5, 2020

kbrock commented Jun 6, 2020 • edited

kbrock commented Jun 6, 2020 • edited

kbrock commented Jun 6, 2020

kbrock commented Jun 10, 2020 • edited

jrafanie Jun 11, 2020

Choose a reason for hiding this comment

jrafanie left a comment

Choose a reason for hiding this comment

kbrock commented Jun 15, 2020 • edited

kbrock commented Jun 17, 2020

kbrock commented Jun 4, 2020 •

edited

coveralls commented Jun 4, 2020 •

edited

kbrock commented Jun 5, 2020 •

edited

kbrock commented Jun 6, 2020 •

edited

kbrock commented Jun 6, 2020 •

edited

kbrock commented Jun 10, 2020 •

edited

kbrock commented Jun 15, 2020 •

edited