test(pdf-service): add pdfDownloadLoader tests #1547
Conversation
| vi.spyOn(PDFDocument.prototype, "embedFont").mockImplementation(function ( | ||
| this: PDFDocument, | ||
| fontData, | ||
| ...args | ||
| ) { | ||
| const adjustedFontData = | ||
| fontData instanceof Buffer ? new Uint8Array(fontData) : fontData; | ||
| return originalEmbedFont.call(this, adjustedFontData, ...args); | ||
| }); | ||
| } |
There was a problem hiding this comment.
I'm a bit confused why we used a function definition here as opposed to an arrow function like above? I have a feeling it has to do with the this binding... but what exactly is that doing?
There was a problem hiding this comment.
Yes, correct. It's not possible to use an arrow function in this case, because we are looking to wrap an instance of the PDFDocument class (which is pdfDoc in fillPdf.server). So we need the this keyword.
In contrast, the other wrapper is wrapping load method of the PDFDocument class down the stack.
|
|
||
| // Convert Buffers to Uint8Array for compatibility with Vitest | ||
| // Vitest expects Uint8Array inputs for certain methods in pdf-lib. | ||
| function wrapPdfDocumentLoad() { |
There was a problem hiding this comment.
nit: would rename these functions something like
| function wrapPdfDocumentLoad() { | |
| function spyOnPdfDocumentLoad() { |
as it wasn't immediately clear what these were doing
There was a problem hiding this comment.
Example: pdfKitBuffer in app/domains/beratungshilfe/services/pdf/index.ts is of type Buffer. Vitest expects is to be Uint8Array (please don't ask me why) which is why it needs to be wrapped like this new Uint8Array(pdfBuffer) before passing it into PDFDocument.load. Instead of touching the code in several places just for this one test, I've decided to wrap on demand inside of the test. So I think the name is quite appropriate. wdyt?
| vi.restoreAllMocks(); | ||
| }); | ||
|
|
||
| it("generates correct PDF for Beratungshilfe", async () => { |
There was a problem hiding this comment.
if you really wanted to get crazy with it, you could consolidate these tests into a single it.each(), with an array of configs for PKH/BerH to test against, but entirely up to you 🤭
There was a problem hiding this comment.
Despite duplication of code, I've decided against this approach - for better readability 🤓
| // Convert Buffers to Uint8Array for compatibility with Vitest | ||
| // Vitest expects Uint8Array inputs for certain methods in pdf-lib. |
There was a problem hiding this comment.
Why does vitest have expectations here?
There was a problem hiding this comment.
I have no answer to this question. Maybe you can shed some light?
For better understanding, see the above answer to @Spencer6497's question.
|
pdfDownloadLoadertests for BerH and PKHexpectstatement for the pdf headers in both BerH and PKH e2e tests