cmorten / superoak

HTTP assertions for Oak made easy via SuperDeno. 🐿 🦕
https://cmorten.github.io/superoak/
MIT License
121 stars 8 forks source link

Tests hang when running with --unstable (Oak 7.3, SuperOak 4.2, Deno 1.9.2) #22

Closed avihavai closed 3 years ago

avihavai commented 3 years ago

Issue

Hi! When running Deno superoak tests with --unstable flag in recent Deno / Oak -versions, the tests get stuck.

Setup:

Details

Example code:

import { Application } from "https://deno.land/x/oak@v7.3.0/mod.ts";
import { superoak } from "https://deno.land/x/superoak@4.2.0/mod.ts";

const app = new Application();

const greet = (context) => {
  context.response.body = "Hello world!";
};

app.use(greet);

Deno.test("Response is 'Hello world!'", async () => {
  const response = await superoak(app);
  await response
    .get("/")
    .expect("Hello world!");
});

When running the tests with deno test --allow-net --unstable app.js, the tests get stuck. However, with deno test --allow-net app.js, the tests can be successfully run. This might have something to do with the native HTTP server enabled in oak when using the --unstable-flag, but have not looked into this in detail. Oak can be normally used with the --unstable flag though.

cmorten commented 3 years ago

Hi @avihavai 👋

Thanks for raising this - I suspect, as you allude to, this is due to Oak now switching to native HTTP bindings under the hood when —unstable is provided for one of the recent ^1.9.0 Deno versions.

I’ve got a feeling a simple feature detection + adjustment to call signature should do it, but needs some more investigation. Until then will say that we support the “old” Oak server but not the new unstable version just yet ( though your reasons for using unstable may well be for other libs in your server which this doesn’t then help with 😅 ).

If you’re up for investigating then really appreciate contributions, otherwise may start looking at it today / tomorrow, workload permitting!

cmorten commented 3 years ago

Hmm, just given a quick spike and my issue atm seems to be that the abort controller doesn't seem to closing the server as expected - will continue to investigate!

avihavai commented 3 years ago

Hi and thanks! :). Out of curiosity, I took a quick peek at SuperDeno + Opine, and the --unstable+hang -issue does not appear there. Unfortunately, I'm quite packed at work at the moment and can't promise to look into this in the near future. Regarding the versions, I'm forced to use --unstable due to other libraries at the moment -- but, can jump back to an older Oak-version for now. Thanks for the effort with the library!

cmorten commented 3 years ago

I think I've found a bug in Oak - the closeServer example doesn't work as intended when using the native bindings, raising an issue there and might take a look at fixing that 🤞

avihavai commented 3 years ago

Nice! Looking forward to a fix! Great work!

cmorten commented 3 years ago

See https://github.com/oakserver/oak/issues/304

cmorten commented 3 years ago

Issue on Oak was closed by https://github.com/oakserver/oak/commit/a7e053cad739b6778683353b137c4379849df1e0. Awaiting a new version to be released so we can test to confirm everything is working.

cmorten commented 3 years ago

oak@7.4.0 has been released but appears to have a similar ( well subtly different ) error which prevents us from using superoak as desired.

See https://github.com/oakserver/oak/issues/314

cmorten commented 3 years ago

Suspecting https://github.com/denoland/deno/issues/10508 may be related to some of the issues find when try to work around Oak’s current state

jsjoeio commented 3 years ago

Looks like https://github.com/denoland/deno/issues/10508#issuecomment-854251753 has been fixed and closed!

asos-craigmorten commented 3 years ago

Raised https://github.com/oakserver/oak/pull/389 which think should resolve this issue.

cmorten commented 3 years ago

Merged 🎉 expect to be fixed next Oak release 🙃

asos-craigmorten commented 3 years ago

Fixed in latest version upstream https://deno.land/x/oak@v9.0.1