cmorten / superoak

HTTP assertions for Oak made easy via SuperDeno. šŸæ šŸ¦•
https://cmorten.github.io/superoak/
MIT License
121 stars 8 forks source link

AssertionError: Test case is leaking async ops. #6

Closed viztastic closed 4 years ago

viztastic commented 4 years ago

Issue

image

Setup:

deno 1.1.0
v8 8.4.300
typescript 3.9.2
oak version: v5.3.0

Getting the following error (screenshot above)

AssertionError: Test case is leaking async ops.
Before:
  - dispatched: 7
  - completed: 6
After:
  - dispatched: 23
  - completed: 23

Am I meant to chain a .end() to the last request in each test file?

asos-craigmorten commented 4 years ago

Hey @viztastic!

You shouldnā€™t need to call .end(), let me know if you find you have to and will investigate.

The error implies that you have a promise/async call that has been dispatched prior to the test but hasnā€™t completed (see your before section of the error). Everything is resolved correctly once the test is completed.

This leads me to suspect that you have an async code running before the test youā€™ve shared and it hasnā€™t been resolved properly. I would check anything you are importing for this test, or any tests that are being run before this test - I donā€™t think this test is at fault.

Let me know if need further help debugging!

viztastic commented 4 years ago

Appreciate the feedback @asos-craigmorten . Took a deeper look and it seems like lines 37-42 of my app.ts is causing the issue

image

My intention there was to handle cleanup (e.g. close open db connections) before the app exits, similar to what terminus did in express land. Not sure if there's a way to let both co-exist or perhaps what I'm doing is an anti-pattern with respect to oak/deno ??

Totally respect this might no longer be a super-oak issue. Just trying to figure out how to get them to play along.

asos-craigmorten commented 4 years ago

Afaik what you have implemented seems sound, it is certainly good practice to clear up post shutdown.

Generally I would advise (as tends to be common practice with supertest which these libraries are ported from) that you separate the concerns of app route and middleware logic from the configuration and start (listen) of the app as superdeno/superoak actually handles the starting and stopping of the server for you.

If youā€™re implementation is quite coupled to the starting of the server or you have quite bespoke listen implementation then you can also pass a url to superoak.

+/- not having it 100% clear how the signal, import and Deno.test are all playing together, you might be better off using the url approach otherwise you may be running the server twice? Note will then also need to handle closing the server yourself if use the url form.

Not 100% familiar with Denoā€™s signal implementation so Iā€™m afraid I canā€™t advise on that one... Iā€™ll have a read! Iā€™m guessing the signal stream is in effect an unresolved promise being awaited on until a signal is finally sent, perhaps when the test ends? Hence why the started async op before the test and would explain why it is complete at the end?

Perhaps you could extract the abort controller and signal handling to a separate entrypoint file which could import the app? You could then test the app in isolation?

Certainly reach out to the Oak community to sanity check the best way to achieve the signal handling - let me know if find anything out!

asos-craigmorten commented 4 years ago

Another avenue you can explore is testing your Oak application using the app.handle() API - I've got an example on the SuperDeno repo's examples.

For now I'm going to close this as it's not an issue with SuperOak per say - please do update this though as / if / when you find a solution so the community can use it as a reference šŸ˜„

asbjornu commented 3 years ago

I had this problem and it was solved by dropping superoak and using superdeno with the app.handle:

import { superdeno } from "https://deno.land/x/superdeno@4.2.1/mod.ts";
import app from "./app.ts";

Deno.test("it should return HTML", async () => {
  await superdeno(app.handle.bind(app))
    .get("/")
    .expect(200)
    .expect("Content-Type", "text/html; charset=utf-8");
});