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 case showing persistent connection problem. #158

Closed
wants to merge 1 commit into from
Closed

Test case showing persistent connection problem. #158

wants to merge 1 commit into from

Conversation

jakecobb
Copy link
Contributor

Test cases for tntim96/JSCover/#91

The single request tests will pass, but the consecutive request tests will fail.

@jakecobb
Copy link
Contributor Author

In some sense the real problem here is that the Proxy-Connection: keep-alive header is getting passed through, causing the remote server to use a persistent connection. Updated the test to show header-based behavior and one that forces persistent connection attempts. Maybe we only want to fix the header-based tests as the other behavior would be a bug in the remote server (since HTTP/1.0 is non-persistent by default).

@tntim96
Copy link
Owner

tntim96 commented Aug 14, 2014

Thanks for looking into this. Yes, after reviewing the code I came to the same conclusion. The server is ignoring the HTTP 1.0 protocol, but I've removed that proxy header anyway. You can download that build from here. Let me know if that works.

@jakecobb
Copy link
Contributor Author

Thanks for the reply. The server is actually doing the right thing; HTTP/1.0 just defaults to non-persistent but the header overrides that. Proxy-Connection is non-standard but treated the same as Connection.

Your fix is incomplete because it only affects GET request with Javascript files, so it doesn't make any failing tests in this pull request pass (I only tested non-JS GET and POST). However, I rebased on top of your changes and was able to make the remaining fixes. Pull request #159 includes your most recent changes, these test cases, my fixes and some additional unit tests for the new code. It passes all tests and coverage checks for ant pre-commit.

@jakecobb jakecobb closed this Aug 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants