Notifying @Yaffle, @odinho, and @zqzhang. (Learn how reviewing works.)

View the complete job log.

Firefox (nightly channel)

Testing web-platform-tests at revision eec8e41
Using browser at version BuildID 20170329100319; SourceStamp 272ce6c2572164f5f6a9fba2a980ba9ccf50770c
Starting 10 test iterations
All results were stable

All results

View the complete job log.

Chrome (unstable channel)

Testing web-platform-tests at revision eec8e41
Using browser at version 59.0.3053.3 dev
Starting 10 test iterations
All results were stable

All results

In the W3C's #testing IRC channel, @jgraham wrote:

unspecified behaviour seems like exactly the kind of thing that we should be
testing because it might cause interop problems

I'm not so sure about this. @RByers I know interoperability has a special place
in your heart. Do you think WPT ought to maintain tests for conditions that are
explicitly forbidden by the standards? Does it make sense to hold browsers
accountable to these cases?

I'm not so sure about this. @RByers I know interoperability has a special place
in your heart. Do you think WPT ought to maintain tests for conditions that are
explicitly forbidden by the standards? Does it make sense to hold browsers
accountable to these cases?

Yeah, I think we should have negative tests where practical. Eg. we've talked about landing tests which validate a given API does NOT exist (eg. when it's been removed from a standard). I'd consider it a chromium bug whenever chromium is doing something explicitly prohibited by a spec. However, in some cases that may be lower priority than what the test is really trying to test - so pragmatically when we find such things it might make the most sense to move them to a separate test case to avoid failures in logically unrelated tests.

Where things get tricky is when a specification leaves something implementation-defined. Often that's a spec bug (just not specified due to omission). Occasionally it's a pragmatic tradeoff where we've accepted that the cost of trying to achieve consensus isn't worth the benefit. In such cases we can't really have a WPT test, though I personally still consider it likely a chromium bug when chromium behaves differently from all other engines in such situations.

Does that help in this case?

I'm in total agreement. The conditions under consideration here are unexpected
in a way that may be fundamentally different, though. In this case, the server
is violating the HTTP protocol. It seems unlikely that any W3C/WhatWG
specification would (or could) account for violations in their dependent
specifications.

Don't get me wrong: I would love it if this effort actually resulted in an
improvement to Chromium. Isolating this behavior was a challenge. I'm just
reluctant to call this an "edge" case--"over the edge" seems more fitting.

I see, thanks. The most important thing here is to try to improve interop of implementations and leverage the knowledge you've gained. So at a minimum, please file a chromium bug describing how Chrome behaved differently from Firefox (and ideally other browsers?). Ping me the bug number and I'll add a comment / label encouraging the network team to look into the options (including removing the opportunity for implementation-defined behavior from the relevant spec).

If there's no per-spec behavior to test here in WPT around this then so be it.

After still further research, I have learned that this behavior is neither a
bug in the tests nor in Chromium but actually a bug in the testing
infrastructure itself.

It seems that Python request handlers share some state. This allows the
execution of one handler to effect the response of independent requests. I've
published a reduced demonstration on the following branch:

master...bocoup:serve-bug-demo

The reduction may make this condition seem kind of arcane, but this basic
behavior is what caused the race condition that this patch is intended to
avoid. In the effected test, the browser creates an EventSource object, and
the server responds with an HTTP status of 204 ("No content"). It happens to
include a body (in violation of the specification), but Chrome is generally
tolerant of this.

The problem is observable if the body happens to be written to the wire in a
separate TCP frame. (This is where the flakiness comes in since this detail is
opaque to the HTTP protocol itself.) In these cases, Chrome interprets the
204 response as "fully received" (in line with the specification), despite the
fact that the Python handler is still running. At this point, when the Python
handler writes the body data, it writes to any open connection it happens to
have.

In case ASCII art would help (and really, when wouldn't it?), here's a
communication flow diagram describing the behavior:

Chrome                                  wptserve

Request1 -----------------------------> -----------. 
|  Request2 -------------------------->            V
|  |                              ___  .-----------------------------.
'<-+---------- TCP Packet #1 --- /     | HTTP/1.1 204 No Content     |
   |                             \___  |                             |
   '<--------- TCP Packet #2 --- /     | Body data (which should not |
                                 \___  | be specified)               |
                                       '-----------------------------'

So this patch is valid, and beyond beyond semantically correct, it does in fact
avoid the race condition. That said, I think it highlights a larger problem
with the tooling. @gsnedders @jgraham could either of you comment on that (and
maybe merge this patch)?

I don't think this patch is valid, since we want in fact be testing 204/205 responses with bodies.

What exactly happens with the second TCP packet? If the connection is still open, Chrome should be able to handle it. Otherwise Chrome should have closed the connection early or some such.

Note also that behavior for 204/205 with bodies is in fact defined by https://fetch.spec.whatwg.org/.

I don't think this patch is valid, since we want in fact be testing 204/205
responses with bodies.

Oh, okay. I have to admit that even after 2 weeks of working with this test,
this was not apparent to me. If the intent is to test the behavior with invalid
responses, then I'd like to create a standalone test that explicitly describes
that condition. However, we won't be able to do this until we've resolved the
bug I've described above.

What exactly happens with the second TCP packet? If the connection is still
open, Chrome should be able to handle it. Otherwise Chrome should have closed
the connection early or some such.

If the connection is still open, Chrome tolerates the extra data (although the
the difference between "accept" and "reject" from the "fetch" spec does not
seem to be appreciable here, since the EventSource spec dictates failure for
all non-200 responses).

The problem with the infrastructure is only appreciable when Chrome closes
the connection. It does this (quite reasonably) if when the HTTP 204/205 response is
segmented at the end of the headers. But in this case, if WPT's server
continues to write to the connection, the data is sent as the response to some
other request. That is what the branch I've shared is intended to demonstrate.

FWIW, what you suggest is fine, but we routinely do invalid things when the behavior is well-defined (as I mentioned, Fetch describes how to handle this). So I think just fixing the infrastructure is fine too and probably more consistent with our overall approach.

@annevk I've opened #5227 to
facilitate a discussion around the under-specified behavior. Because the
resolution there is orthogonal to the behavior that this particular test
intends to exercise and because the current version of this test is not
currently usable by Chromium, I would like to proceed with the patch proposed
here. Would you mind merging it?

LGTM, but I cannot merge this due to Travis.

That's very odd. I've rebased, squashed and force-pushed the result to this branch (original available here); that seems to have fixed the issue.

@jgraham check out the two "check stability" jobs in this build:

https://travis-ci.org/w3c/web-platform-tests/builds/216048041

I can see two separate issues here. First, the script interpreted all the new CSS tests as part of the branch. Second, the output was mot truncated when it reached the TravisCI-imposed limit. Can you think of a reason why either of these things would happen?

Where does the convention of the [ and ] in commit titles come from? Seen it a few times now.

I don't think there is precedence for it in this project. It's something I started to follow when submitting multiple patches with similar intentions whose subject lines would otherwise be indistinguishable (e.g. "Remove duplicate test names"). That's not the case here, but in the absence of any guidelines (and with the characters to spare), I figured it couldn't hurt.

@jgraham It looks like I made an error in my earlier comment:

I can see two separate issues here. First, the script interpreted all the new
CSS tests as part of the branch. Second, the output was mot truncated when it
reached the TravisCI-imposed limit. Can you think of a reason why either of
these things would happen?

The second issue is not actually occurring here. While the TravisCI UI is not rendering the full log, the output itself was correctly truncated, and the job failed due to the script's exit status. So the problem here is limited to the first issue I described.