Skip to content

Conversation

@p8
Copy link
Contributor

@p8 p8 commented Jan 22, 2025

The requirements state that:

Implementations that are not based on a realistic HTTP implementation
will be marked as Stripped.

The Rack tests use webservers (Puma, Unicorn, Falcon) based on realisitic HTTP implementation, and the Rack specification does as well.

So if we are going to mark some frameworks as non realistic because they don't conform to a realistic HTTP implementation, why should Rack be marked as Stripped when it does conform?

The requirements state that:

    Implementations that are not based on a realistic HTTP implementation
    will be marked as Stripped.

See: https://github.com/TechEmpower/FrameworkBenchmarks/wiki/Project-Information-Framework-Tests-Overview#general-test-requirements

The Rack tests use webservers based on realisitic HTTP implementation,
and Rack specification does as well:
See: https://github.com/rack/rack/blob/main/SPEC.rdoc
@joanhey
Copy link
Contributor

joanhey commented Jan 22, 2025

LGTM this change.
It was changed a lo of time ago, without any explication in:
e684b54#diff-33dc9963b3f86ae43192c37d5134b3560ec3f56259b23ce9bf2afdca68458f27R12

@p8
Copy link
Contributor Author

p8 commented Jan 22, 2025

Also frameworks hardcoding the Content-Length should probably be marked as Stripped?

const contentLengthS = "\r\nContent-Length: 13\r\n\r\nHello, World!"

b"HTTP/1.1 200 OK\r\nServer: A\r\nContent-Type: application/json\r\nContent-Length: 27\r\n";

b"HTTP/1.1 200 OK\r\nServer: N\r\nContent-Type: application/json\r\nContent-Length: 27\r\n";

edit: some of these are already marked as stripped.

@msmith-techempower msmith-techempower merged commit a023ee0 into TechEmpower:master Jan 23, 2025
4 checks passed
@p8 p8 deleted the rack/realistic branch January 23, 2025 18:17
@joanhey joanhey mentioned this pull request Jan 31, 2025
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.

3 participants