sstur / nbit

A zero-dependency, strongly-typed web framework for Bun, Node and Cloudflare workers
65 stars 4 forks source link

`Response.file` not working in test? #8

Closed ImLunaHey closed 1 year ago

ImLunaHey commented 1 year ago

Running this test with bun test returns a 404 instead of a 200.

import { createApplication, Request, Response } from '@nbit/bun';
import { expect, test } from 'bun:test';

const home = () => Response.file('public/index.html');

const { defineRoutes, createRequestHandler } = createApplication();

const routes = defineRoutes(app => [
    app.get('/', home),
]);

const requestHandler = createRequestHandler(routes);

test('should return the home page', async () => {
    const request = new Request('http://localhost/');
    const response = await requestHandler(request);
    expect(response.status).toBe(200);
});

Changing home to be this instead allows the test to pass.

const home = () => ({ hello: 'world' });

I know it's not a path issue as I even tried the absolute path and still it doesn't work yet my site is running right now with this so I know it works. 😅

ImLunaHey commented 1 year ago

I dont know if this is a bun bug or a bug with this library. 🤔

sstur commented 1 year ago

OK, I think I know why.

const requestHandler = createRequestHandler(routes) creates a real request handler, not a mock or test one, meaning it will try to serve a real file from the filesystem when you return Response.file(path).

However, when we call createApplication() above, we're not specifying the options root and allowStaticFrom (see example). Without those options, no files are allowed to be served, so all attempts to serve a file will result in a 404 instead.

So really we have two issues here:

  1. The file serving options need to be specified (easy fix)
  2. We're now reading real files from the file system whenever we run our tests (harder issue)

It's not really ideal to be serving real files while running tests. We'd typically mock this, but there's currently no mechanism built in to this framework for mocking stuff like this. I suppose we could try to mock fs.stat() and Bun.file(). That feels a bit low-level, but it might be the only way to do this for now.

For reference, in your tests, when you call const response = await requestHandler(request) the requestHandler function is defined here, which calls routeRequest() which calls toResponse() which calls fromStaticFile() which calls serveFile() which calls statAsync() and Bun.file().

That was a tricky thing to dig up, so I thought I'd leave it here for reference.

I wonder if I should provide a way to override serveFile(), perhaps conditionally (only while running tests), this might be the easiest way to support mocks like this, but I'm not sure the best approach.

Thoughts?

ImLunaHey commented 1 year ago

Oh interesting, thanks for digging into this.

It's not really ideal to be serving real files while running tests.

Agreed.

I wonder if I should provide a way to override serveFile(), perhaps conditionally (only while running tests), this might be the easiest way to support mocks like this, but I'm not sure the best approach.

This sounds like the most reasonable approach. I can't think of any other way this could really be handled.

sstur commented 1 year ago

This is now implemented in v0.11. You can specify a serveFile function in the createApplication config.

Example:

const isTestEnvironment = process.env.IS_TEST === 'true'; // Or whatever you choose to use

const { defineRoutes, createRequestHandler } = createApplication({
  root: '/home/myapp', // Optional; defaults to process.cwd()
  allowStaticFrom: ['public'],
  serveFile: isTestEnvironment
    ? async ({ fullFilePath }) => {
        if (fullFilePath === '/home/myapp/public/file.txt') {
          const mockFileContent = 'Some mock content';
          return new Response(mockFileContent, {
            headers: { 'content-type': 'text/plain' },
          });
        }
        return new Response('Invalid file path', { status: 404 });
      }
    : undefined,
});

const routes = defineRoutes((app) => [
  app.get('/file', async (request) => {
    return Response.file('public/file.txt');
  }),
]);

const requestHandler = createRequestHandler(routes);
const request = new Request('http://localhost/file');
const response = await requestHandler(request);

expect(response.status).toBe(200);
expect(response.headers.get('content-type')).toBe('text/plain');
const body = await response.text();
expect(body).toBe('Some mock content');