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

test(pdf-service): add pdfDownloadLoader tests #1547

Merged
merged 11 commits into from
Dec 12, 2024
96 changes: 96 additions & 0 deletions app/routes/shared/__test__/pdfDownloadLoader.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import type { LoaderFunctionArgs } from "@remix-run/node";
import { PDFDocument } from "pdf-lib";
import { pdfDownloadLoader } from "../pdfDownloadLoader";

// Convert Buffers to Uint8Array for compatibility with Vitest
// Vitest expects Uint8Array inputs for certain methods in pdf-lib.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does vitest have expectations here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no answer to this question. Maybe you can shed some light?

For better understanding, see the above answer to @Spencer6497's question.

function wrapPdfDocumentLoad() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: would rename these functions something like

Suggested change
function wrapPdfDocumentLoad() {
function spyOnPdfDocumentLoad() {

as it wasn't immediately clear what these were doing

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

const originalLoad = PDFDocument.load;
vi.spyOn(PDFDocument, "load").mockImplementation((input, ...args) => {
const adjustedInput =
input instanceof Buffer ? new Uint8Array(input) : input;
return originalLoad.call(PDFDocument, adjustedInput, ...args);
});
}

function wrapEmbedFont() {
const originalEmbedFont = PDFDocument.prototype.embedFont;
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);
});
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


vi.mock("~/services/flow/pruner", () => ({
pruneIrrelevantData: vi
.fn()
.mockResolvedValue({ vorname: "Zoe", nachname: "Müller" }),
}));

describe("pdfDownloadLoader", () => {
beforeAll(() => {
wrapPdfDocumentLoad();
wrapEmbedFont();
});

afterAll(() => {
vi.restoreAllMocks();
});

it("generates correct PDF for Beratungshilfe", async () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🤭

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Despite duplication of code, I've decided against this approach - for better readability 🤓

const mockLoaderArgs: LoaderFunctionArgs = {
request: new Request(
"https://mock-url.de/beratungshilfe/antrag/download/pdf",
{
method: "GET",
headers: new Headers({ Cookie: "mock-session-data" }),
},
),
params: {},
context: {},
};

const response = await pdfDownloadLoader(mockLoaderArgs);
const pdfBuffer = Buffer.from(await response.arrayBuffer());
const pdfDoc = await PDFDocument.load(new Uint8Array(pdfBuffer));
const nameField = pdfDoc
.getForm()
.getTextField("Antragsteller (Name, Vorname ggf Geburtsname)");

expect(response.status).toBe(200);
expect(response.headers.get("Content-Type")).toBe("application/pdf");
expect(pdfDoc.getPageCount()).toBe(4);
expect(nameField.getText()).toBe("Müller, Zoe");
});

it("generates correct PDF for Prozesskostenhilfe", async () => {
const mockLoaderArgs: LoaderFunctionArgs = {
request: new Request(
"https://mock-url.de/prozesskostenhilfe/formular/download/pdf",
{
method: "GET",
headers: new Headers({ Cookie: "mock-session-data" }),
},
),
params: {},
context: {},
};

const response = await pdfDownloadLoader(mockLoaderArgs);
const pdfBuffer = Buffer.from(await response.arrayBuffer());
const pdfDoc = await PDFDocument.load(new Uint8Array(pdfBuffer));
const nameField = pdfDoc
.getForm()
.getTextField("Name Vorname ggf Geburtsname");

expect(response.status).toBe(200);
expect(response.headers.get("Content-Type")).toBe("application/pdf");
expect(pdfDoc.getPageCount()).toBe(9);
expect(nameField.getText()).toBe("Müller, Zoe");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,4 @@ async function startAbgabe(page: Page) {

expect(newTabResponse).not.toBeUndefined();
expect(newTabResponse?.status()).toBe(200);
expect(await newTabResponse?.headerValue("content-type")).toBe(
"application/pdf",
);
}
3 changes: 0 additions & 3 deletions tests/e2e/domains/prozesskostenhilfe/formular/abgabe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,4 @@ export async function startAbgabe(page: Page) {

expect(newTabResponse).not.toBeUndefined();
expect(newTabResponse?.status()).toBe(200);
expect(await newTabResponse?.headerValue("content-type")).toBe(
"application/pdf",
);
}
Loading