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

Re-enable flaky fetch tests #1228

Closed
ry opened this issue Nov 27, 2018 · 6 comments · Fixed by #5803
Closed

Re-enable flaky fetch tests #1228

ry opened this issue Nov 27, 2018 · 6 comments · Fixed by #5803
Labels
bug Something isn't working correctly good first issue Good for newcomers
Milestone

Comments

@ry
Copy link
Member

ry commented Nov 27, 2018

Note the issue was only appearing on Windows. So ideally someone on Windows would debug this. The first set is to uncomment the tests and see if you can repeat the failure.

deno/js/fetch_test.ts

Lines 50 to 161 in 570269b

// TODO(ry) The following tests work but are flaky. There's a race condition
// somewhere. Here is what one of these flaky failures looks like:
//
// test fetchPostBodyString_permW0N1E0R0
// assertEqual failed. actual = expected = POST /blah HTTP/1.1
// hello: World
// foo: Bar
// host: 127.0.0.1:4502
// content-length: 11
// hello world
// Error: actual: expected: POST /blah HTTP/1.1
// hello: World
// foo: Bar
// host: 127.0.0.1:4502
// content-length: 11
// hello world
// at Object.assertEqual (file:///C:/deno/js/testing/util.ts:29:11)
// at fetchPostBodyString (file
/*
function bufferServer(addr: string): deno.Buffer {
const listener = deno.listen("tcp", addr);
const buf = new deno.Buffer();
listener.accept().then(async conn => {
const p1 = buf.readFrom(conn);
const p2 = conn.write(
new TextEncoder().encode(
"HTTP/1.0 404 Not Found\r\nContent-Length: 2\r\n\r\nNF"
)
);
// Wait for both an EOF on the read side of the socket and for the write to
// complete before closing it. Due to keep-alive, the EOF won't be sent
// until the Connection close (HTTP/1.0) response, so readFrom() can't
// proceed write. Conversely, if readFrom() is async, waiting for the
// write() to complete is not a guarantee that we've read the incoming
// request.
await Promise.all([p1, p2]);
conn.close();
listener.close();
});
return buf;
}
testPerm({ net: true }, async function fetchRequest() {
const addr = "127.0.0.1:4501";
const buf = bufferServer(addr);
const response = await fetch(`http://${addr}/blah`, {
method: "POST",
headers: [["Hello", "World"], ["Foo", "Bar"]]
});
assertEqual(response.status, 404);
assertEqual(response.headers.get("Content-Length"), "2");
const actual = new TextDecoder().decode(buf.bytes());
const expected = [
"POST /blah HTTP/1.1\r\n",
"hello: World\r\n",
"foo: Bar\r\n",
`host: ${addr}\r\n\r\n`
].join("");
assertEqual(actual, expected);
});
testPerm({ net: true }, async function fetchPostBodyString() {
const addr = "127.0.0.1:4502";
const buf = bufferServer(addr);
const body = "hello world";
const response = await fetch(`http://${addr}/blah`, {
method: "POST",
headers: [["Hello", "World"], ["Foo", "Bar"]],
body
});
assertEqual(response.status, 404);
assertEqual(response.headers.get("Content-Length"), "2");
const actual = new TextDecoder().decode(buf.bytes());
const expected = [
"POST /blah HTTP/1.1\r\n",
"hello: World\r\n",
"foo: Bar\r\n",
`host: ${addr}\r\n`,
`content-length: ${body.length}\r\n\r\n`,
body
].join("");
assertEqual(actual, expected);
});
testPerm({ net: true }, async function fetchPostBodyTypedArray() {
const addr = "127.0.0.1:4503";
const buf = bufferServer(addr);
const bodyStr = "hello world";
const body = new TextEncoder().encode(bodyStr);
const response = await fetch(`http://${addr}/blah`, {
method: "POST",
headers: [["Hello", "World"], ["Foo", "Bar"]],
body
});
assertEqual(response.status, 404);
assertEqual(response.headers.get("Content-Length"), "2");
const actual = new TextDecoder().decode(buf.bytes());
const expected = [
"POST /blah HTTP/1.1\r\n",
"hello: World\r\n",
"foo: Bar\r\n",
`host: ${addr}\r\n`,
`content-length: ${body.byteLength}\r\n\r\n`,
bodyStr
].join("");
assertEqual(actual, expected);
});
*/

@ry ry added the bug Something isn't working correctly label Nov 27, 2018
@ry ry added this to the v0.3 milestone Nov 27, 2018
@kt3k
Copy link
Member

kt3k commented Jan 19, 2019

duplicate of #1219 ?

@ry ry modified the milestones: v0.3, v0.4 Feb 12, 2019
@nstott
Copy link
Contributor

nstott commented Nov 2, 2019

These are flakey on mac as well now, not just windows

@ry
Copy link
Member Author

ry commented Feb 24, 2020

Feb 24, 2020: Still an issue. If we could just skip the tests on windows, that would be great.

@ry ry added the good first issue Good for newcomers label Feb 24, 2020
@mitch292
Copy link
Contributor

Commented on the PR I submitted for this as well, but the test for fetchRequest in fetch_tests.ts is failing in the build on the latest macOs, but successful on my local. Both running 10.15.4, having a little trouble debugging. Any pointers would be appreciated.

Here is the output from the build for the test.

 fetchRequest
AssertionError: Values are not equal:


    [Diff] Actual / Expected

Unit tests failed

+   "POST /blah HTTP/1.1
+   hello: World
+   foo: Bar
+   accept: */*
+   user-agent: Deno/1.0.2
+   accept-encoding: gzip, br
+   host: 127.0.0.1:4501
+   
+   "
-   ""

@mitch292
Copy link
Contributor

Ah my bad, I see in the original comment and #1535 that this was the output that spurred original issue. Going to close my PR, but experienced today on macOS.

@mitch292
Copy link
Contributor

Apologies for all the above comments. PR's back open. I wasn't resolving the response body properly in my original PR.

@ry ry closed this as completed in #5803 May 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly good first issue Good for newcomers
Projects
None yet
4 participants