-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
chore: disable 'test-http-content-length.js` test #20344
Conversation
The problem may lie in the node-compat test I added. |
The previous behavior of Deno |
@osddeitf could you point me to the relevant part that should fail but didn't? Maybe there's another bug hiding there that I could fix. |
This is my fix attempt:
I'll pull the latest code and try to rerun the test again, before, it worked totally normal for me (no flakiness), it may also because of other changes to Node compatibility layer. |
@osddeitf ideally we'd just use test file from Node without any changes, if it fails because of discrepancies in sent headers we can remove unnecessary headers from being sent. |
I know, that's why I changed that in the first commit mentioned above. The test passed as I expected. |
Alright, let's then work on fixing these assertion errors! |
Thank you! We still have some time till v1.37, I'll do my part too when I have some free time. |
Sounds good, I will then disable the test for now as it is causing flakes on CI |
8d65b34
to
4a78de8
Compare
Content-length
header instea…There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@bartlomieju let me report on what I found (again, sorry if it's too long):
So I surmise that inside the test, the server get closed (when |
In Nodejs docs server.close([callback]):
So I think this is indeed an issue of |
…d of
Transfer-Encoding: chunked
(#20127)"This reverts commit c2547ba.
After merging that PR there's quite a lot of flakiness in Node compat tests that
fail like so:
CC @osddeitf