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

Support Fastify #2

Closed
SimenB opened this issue Jun 5, 2020 · 6 comments · Fixed by #3
Closed

Support Fastify #2

SimenB opened this issue Jun 5, 2020 · 6 comments · Fixed by #3

Comments

@SimenB
Copy link

SimenB commented Jun 5, 2020

We're currently migrating from express to fastify, would love to keep using this library!

https://www.fastify.io/

@remcohaszing
Copy link
Owner

Thank you for your interest!

I gave it a try, but as you can see in the failing tests of the pull request, starting a fastify instance a second time, even after closing it properly, fails.

Also I’m not too happy about the framework detection mechanism I came up with.

I don’t think I’m going to dive much deeper into support for Fastify, but if someone else manages to fix it, I’ll gladly merge it.

@SimenB
Copy link
Author

SimenB commented Jun 6, 2020

In their docs, they have a supertest example which gets passed the server. Would that work, somehow?

https://www.fastify.io/docs/v3.0.x/Testing/#example-1

Seems we'd have to do ready and close ourselves regardless, which somewhat defeats the point... 🙁

@remcohaszing
Copy link
Owner

Seems we'd have to do ready and close ourselves regardless, which somewhat defeats the point... 🙁

That part has already been abstracted away. I want this library to make the test code as simple as possible. 😉


The issue is that fastify instances can’t be reused. I.e. the following code won’t work if multiple tests are present.

// app.js
import * as Fastify from 'fastify'

const app = Fastify();
app.get('/', async (request, reply) => {
  reply.send({ hello: 'world' });
});
export default app;

// app.test.js
import { request, setTestApp } from 'axios-test-instance'
import app from './app'

beforeEach(() => setTestApp(app))

The following examples do work with the current PR. This is the same pattern as is used in their supertest example.

// createApp.js
import * as Fastify from 'fastify'

export default function createApp() {
  const app = Fastify();
  app.get('/', async (request, reply) => {
    reply.send({ hello: 'world' });
  });
  return app;
}

// createApp.test.js
import { request, setTestApp } from 'axios-test-instance'
import createApp from './createApp'

beforeEach(() => setTestApp(createApp()))
// createApp.js
import * as Fastify from 'fastify'

export default function createApp() {
  const app = Fastify();
  app.get('/', async (request, reply) => {
    reply.send({ hello: 'world' });
  });
  return app;
}

// createApp.test.js
import { request, setTestApp } from 'axios-test-instance'
import createApp from './createApp'

beforeAll(() => setTestApp(createApp()))

Because jest creates new instances for each test file when module isolation isn’t disabled, the following should also just work with jest.

// app.js
import * as Fastify from 'fastify'

const app = Fastify();
app.get('/', async (request, reply) => {
  reply.send({ hello: 'world' });
});
export default app;

// app.test.js
import { request, setTestApp } from 'axios-test-instance'
import app from './app'

beforeAll(() => setTestApp(app))

If the scenario above meets your requirements, I’ll gladly finalize my changes and release them with a footnote about the quirks of using this library with fastify.

@SimenB
Copy link
Author

SimenB commented Jun 11, 2020

Yeah, that should cover my use cases, thanks! I think documenting that behavior should be enough

@remcohaszing
Copy link
Owner

This has been released as 3.1.0.

@SimenB
Copy link
Author

SimenB commented Jun 14, 2020

Thanks! It works great, except for https://github.com/remcohaszing/axios-test-instance/pull/3/files#r439827863

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants