8000 fix test numbers by derekargueta · Pull Request #511 · nodejs/http-parser · GitHub
[go: up one dir, main page]
Skip to content
This repository was archived by the owner on Nov 6, 2022. It is now read-only.

fix test numbers #511

Closed
wants to merge 1 commit into from
Closed

fix test numbers #511

wants to merge 1 commit into from

Conversation

derekargueta
Copy link
Contributor

42 is duplicated in request tests, and 20 is duplicated in response tests

@bnoordhuis
Copy link
Member

Thanks. I'd be happy to merge this but if you want you can just remove all the defines, they're not actually used. IIRC, they were added as a kind of (not very effective) self-documentation.

@derekargueta
Copy link
Contributor Author

@bnoordhuis sure I can take care of that clean up later tonight. Thanks for taking a look!

@derekargueta
Copy link
Contributor Author

@bnoordhuis There seems to actually be a few tests that rely on these defines to index into the test case arrays. For example: https://github.com/nodejs/http-parser/blob/master/test.c#L4334

After removing the defines I got 20 compilation errors

cc  -I. -DHTTP_PARSER_STRICT=1  -Wall -Wextra -Werror -O0 -g  -c test.c -o test_g.o
test.c:4258:38: error: use of undeclared identifier 'NO_HEADERS_NO_BODY_404'
  test_message_count_body(&responses[NO_HEADERS_NO_BODY_404]);
                                     ^
test.c:4259:38: error: use of undeclared identifier 'TRAILING_SPACE_ON_CHUNKED_BODY'
  test_message_count_body(&responses[TRAILING_SPACE_ON_CHUNKED_BODY]);
                                     ^
test.c:4296:25: error: use of undeclared identifier 'TRAILING_SPACE_ON_CHUNKED_BODY'
  test_scan( &responses[TRAILING_SPACE_ON_CHUNKED_BODY]
                        ^
test.c:4297:25: error: use of undeclared identifier 'NO_BODY_HTTP10_KA_204'
           , &responses[NO_BODY_HTTP10_KA_204]
                        ^
test.c:4298:25: error: use of undeclared identifier 'NO_REASON_PHRASE'
           , &responses[NO_REASON_PHRASE]
                        ^
test.c:4302:25: error: use of undeclared identifier 'BONJOUR_MADAME_FR'
  test_scan( &responses[BONJOUR_MADAME_FR]
                        ^
test.c:4303:25: error: use of undeclared identifier 'UNDERSTORE_HEADER_KEY'
           , &responses[UNDERSTORE_HEADER_KEY]
                        ^
test.c:4304:25: error: use of undeclared identifier 'NO_CARRIAGE_RET'
           , &responses[NO_CARRIAGE_RET]
                        ^
test.c:4498:24: error: use of undeclared identifier 'GET_NO_HEADERS_NO_BODY'
  test_scan( &requests[GET_NO_HEADERS_NO_BODY]
                       ^
test.c:4499:24: error: use of undeclared identifier 'GET_ONE_HEADER_NO_BODY'
           , &requests[GET_ONE_HEADER_NO_BODY]
                       ^
test.c:4500:24: error: use of undeclared identifier 'GET_NO_HEADERS_NO_BODY'
           , &requests[GET_NO_HEADERS_NO_BODY]
                       ^
test.c:4504:24: error: use of undeclared identifier 'POST_CHUNKED_ALL_YOUR_BASE'
  test_scan( &requests[POST_CHUNKED_ALL_YOUR_BASE]
                       ^
test.c:4505:24: error: use of undeclared identifier 'POST_IDENTITY_BODY_WORLD'
           , &requests[POST_IDENTITY_BODY_WORLD]
                       ^
test.c:4506:24: error: use of undeclared identifier 'GET_FUNKY_CONTENT_LENGTH'
           , &requests[GET_FUNKY_CONTENT_LENGTH]
                       ^
test.c:4510:24: error: use of undeclared identifier 'TWO_CHUNKS_MULT_ZERO_END'
  test_scan( &requests[TWO_CHUNKS_MULT_ZERO_END]
                       ^
test.c:4511:24: error: use of undeclared identifier 'CHUNKED_W_TRAILING_HEADERS'
           , &requests[CHUNKED_W_TRAILING_HEADERS]
                       ^
test.c:4512:24: error: use of undeclared identifier 'CHUNKED_W_NONSENSE_AFTER_LENGTH'
           , &requests[CHUNKED_W_NONSENSE_AFTER_LENGTH]
                       ^
test.c:4516:24: error: use of undeclared identifier 'QUERY_URL_WITH_QUESTION_MARK_GET'
  test_scan( &requests[QUERY_URL_WITH_QUESTION_MARK_GET]
                       ^
test.c:4517:24: error: use of undeclared identifier 'PREFIX_NEWLINE_GET'
           , &requests[PREFIX_NEWLINE_GET ]
                       ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
make: *** [test_g.o] Error 1

I think the defines would be preferable to hard-coded magic array indices, although it is pretty fragile :/

bnoordhuis pushed a commit that referenced this pull request May 5, 2020
@derekargueta @bnoordhuis
Fix test numbers
805a0d1 
PR-URL: #511
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@bnoordhuis
Copy link
Member

You're right. I missed those, sorry. Landed in 805a0d1, thanks!

@bnoordhuis bnoordhuis closed this May 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@derekargueta @bnoordhuis
0