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

test against the http2 compatibility layer #1454

Open
jonathanong opened this issue Apr 29, 2020 · 5 comments
Open

test against the http2 compatibility layer #1454

jonathanong opened this issue Apr 29, 2020 · 5 comments

Comments

@jonathanong
Copy link
Member

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.

@ognjenjevremovic
Copy link

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! 🎉

@miwnwski
Copy link
Member

miwnwski commented Mar 2, 2021

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.

@ognjenjevremovic
Copy link

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!

@jkomyno
Copy link

jkomyno commented Apr 16, 2021

Hi @ognjenjevremovic, any update with this?

@ognjenjevremovic
Copy link

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.

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

No branches or pull requests

4 participants