elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.64k stars 8.22k forks source link

[Reporting] CI test coverage for generating PDFs (both print-optimized and preserve layout) #117814

Open jloleysens opened 3 years ago

jloleysens commented 3 years ago

Summary

Reporting has the capability of generating both PNGs and PDFs. We have coverage for generating PNGs by running a comparison against a baseline image but do not have the same coverage for PDFs. We should add:

Additional context

elasticmachine commented 3 years ago

Pinging @elastic/kibana-reporting-services (Team:Reporting Services)

tsullivan commented 3 years ago

@jloleysens

We need to find a dependency (or patch a dependency) that will run on all (Windows too?) main architectures Kibana is developed on.

An alternative idea: what if we define a new CI runner for testing PDF generation, and configure it to only run on the architecture that is supported by the dependency? The downside of this idea is that we would lose cross-architecture coverage. But as long as we have cross-architecture coverage of PNG generation, we should be just fine.

jloleysens commented 2 years ago

I've found what could be a pure JS way of creating PNGs to compare against a baseline: https://www.npmjs.com/package/pureimage. The reasoning for that lib is specifically to avoid relying on the backing native dependencies (Cairo) and as a result is slower, but useful for tests that need to be really portable.

We could take this code: https://github.com/k-yle/pdf-to-img/blob/8998c3008c0be8394f18cea5b1503c059ee066a6/src/index.ts#L57

And replace Node.js canvas implementation with the pure JS canvas from pureimage.

jloleysens commented 2 years ago

I've found what could be a pure JS way of creating PNGs to compare against a baseline: https://www.npmjs.com/package/pureimage. The reasoning for that lib is specifically to avoid relying on the backing native dependencies (Cairo) and as a result is slower, but useful for tests that need to be really portable.

We could take this code: https://github.com/k-yle/pdf-to-img/blob/8998c3008c0be8394f18cea5b1503c059ee066a6/src/index.ts#L57

And replace Node.js canvas implementation with the pure JS canvas from pureimage.

Tested this out recently and the Canvas replacement does not correctly implement the Canvas API contracts so it did not work OOO 😞

tsullivan commented 2 years ago

The team wants to pick up this topic again. It's in our goals to have test coverage for PDF, because some feature we create need to be handed-off to other teams for ownership of an app-level feature, and the code that is handed off must have really good test coverage.

See https://github.com/elastic/kibana/issues/116708 for why the PDF test coverage was temporarily removed. We need to find a dependency (or patch a dependency) that will run on all (Windows too?) main architectures Kibana is developed on.

Another alternative would be to create a new project repo (or sub-repo) for tests that depend on a single-platform binary. It would unblock us from figuring out how to use our preferred package.

A couple downsides:

cc @brianseeders

elasticmachine commented 2 years ago

Pinging @elastic/kibana-app-services (Team:AppServicesSv)

tsullivan commented 2 months ago

https://github.com/moshensky/pdf-visual-diff is a library which relies on pdf.js for converting PDFs to images and jimp for image comparison, both of which are JavaScript libraries that should work on ARM architecture. Let's look into using this to solve our testing needs.