koa icon indicating copy to clipboard operation
koa copied to clipboard

test against the http2 compatibility layer

Open jonathanong opened this issue 5 years ago • 5 comments

although we don't support the new HTTP2 APIs, we should run all our tests against both the http and http2 modules, using only the HTTP/1 API

easiest method would be to set an env var like KOA_TEST_HTTP_MODULE=http and run tests with each option.

jonathanong avatar Apr 29 '20 05:04 jonathanong

Hi @jonathanong 👋

I see this issue is open for quite some time now (with no work being logged / linked to the issue) and labeled as a help wanted. I didn't make PR previously to koa (however, I find myself familiar with Node built-in http modules and express like frameworks). Do you think this issue would be a good welcoming issue to the first time contributors to the project? I might give this one a try if so.

Can you confirm that issue is still open (with no work being done) and is up for grabs? 🙂 If so, can you please provide more info on where one might start in order to successfully implement this?

Many thanks! 🎉

ognjenjevremovic avatar Feb 27 '21 19:02 ognjenjevremovic

Hi @ognjenjevremovic,

This issue is open for grabs. My personal suggestion is that if you want to make a contributing PR just go for it. It'll get reviewed once pushed.

To get started with HTTP/2 tests I suggest taking a look at available test and mimic these using http2 instead. Contrary to HTTP/1 which is automatically selected when calling Koa#listen you would need to call Koa#callback to get a function that quacks like a requestListener and do some tests against node.js's http2 built-in module.

miwnwski avatar Mar 02 '21 08:03 miwnwski

Thank you for getting back to me @miwnwski. I wasn't sure if this issue was to include only test changes against http2 API, or the lib needed to go through changes as well.

I will provide a PR some time this week. I'll post update once I have some. Cheers!

ognjenjevremovic avatar Mar 02 '21 11:03 ognjenjevremovic

Hi @ognjenjevremovic, any update with this?

jkomyno avatar Apr 16 '21 18:04 jkomyno

Hey @jkomyno, apologize for my late response. I didn't have much success with this (as far as I remember). I can't remember why, but I think it was something regarding on how I approached the issue altogether.

I think I tried merging the files at first, with both tests included (and reading through the command line arguments, when starting the tests to determine which set of tests to run). I guess the tests should have been separated into different directories / files. I might give this another short, next week (this time, posting the status update on time).

If you wanted to give this issue a shot instead, please do feel free to do so as I'm not actively working on it.

ognjenjevremovic avatar Apr 22 '21 18:04 ognjenjevremovic