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

hyper: customize test1274 to how hyper unfolds headers #9217

Closed
wants to merge 1 commit into from

Conversation

seanmonstar
Copy link
Contributor

@seanmonstar seanmonstar commented Jul 28, 2022

I tried following the way I see some other test data files do this, but after I made the changes locally and re-run ./runtests.pl 1274, it still sees the old output. Perhaps there's a step in make test that I'm skipping locally?

@bagder
Copy link
Member

bagder commented Jul 28, 2022

Hm, curious. I agree that it looks weird as the change seems fine. I need to look into it.

@bagder bagder added the Hyper label Jul 28, 2022
@bagder bagder self-assigned this Jul 28, 2022
@bagder
Copy link
Member

bagder commented Aug 10, 2022

The issue here is quite simply that the built-in code does no unfolding of the headers that it stores with -D, the only unfolding it does is when storing the headers for the headers API.

@bagder
Copy link
Member

bagder commented Aug 10, 2022

I propose we make a conditional in the test case, like this:

index e789a7232..785f74768 100644
--- a/tests/data/test1274
+++ b/tests/data/test1274
@@ -59,17 +59,25 @@ Accept: */*
 
 </protocol>
 <file name="log/out%TESTNUMBER">
 HTTP/1.1 200 OK
 Date: Tue, 09 Nov 2010 14:49:00 GMT
+%if hyper
+Server: test-server/ fake folded^M
+%else
 Server: test-server/
  fake
  folded
+%endif
 Last-Modified: Tue, 13 Jun 2000 12:10:00 GMT
 ETag: "21025-dc7-39462498"
 Content-Length: 6
+%if hyper
+Connection: close^M
+%else
 Connection:                                              
    close
+%endif
 
 </file>
 </verify>
 </testcase>

@bagder bagder added the tests label Aug 10, 2022
@seanmonstar
Copy link
Contributor Author

seanmonstar commented Aug 10, 2022

I think that's what I did in this PR, but maybe I'm missing an important detail.

@bagder
Copy link
Member

bagder commented Aug 10, 2022

Ah, yes now I see it, you edited the <data> part with ifdefs which is the part that the server uses to serve data with - although it also used for checking that what was received (stored really) was exactly what was sent. In this case, with hyper, what is sent is not exactly what is stored really.

New edit/proposal attached. Gzipped to make the line endings stick. This version makes the test work for me.

test1274.gz

@bagder
Copy link
Member

bagder commented Aug 10, 2022

If you have no objections I can merge it like that.

@seanmonstar
Copy link
Contributor Author

seanmonstar commented Aug 10, 2022

Ohhh, I see now. I had done them in the wrong section. Thanks for clarifying! Sure, no objections here :)

@bagder bagder closed this in d6010c2 Aug 10, 2022
@bagder
Copy link
Member

bagder commented Aug 10, 2022

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

2 participants