-
Notifications
You must be signed in to change notification settings - Fork 323
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
Improve HTTP tests #7847
Improve HTTP tests #7847
Conversation
distribution/lib/Standard/Base/0.0.0-dev/src/Network/HTTP/Request_Body.enso
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The title says "Improve HTTP tests" - I didn't know what to expected, but to my disappointment I don't see any simplification of the testing infrastructure. Setting it up still remains complicated and as a result (almost) nobody executes these tests locally.
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Set; | ||
import java.util.regex.Matcher; | ||
import java.util.regex.Pattern; | ||
import org.apache.commons.text.StringEscapeUtils; | ||
|
||
public class DummyHandler implements HttpHandler { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this class called DummyHandler
?
@@ -8,6 +8,7 @@ import Standard.Base.Network.HTTP.Request_Body.Request_Body | |||
import Standard.Base.Network.HTTP.Request_Error | |||
import Standard.Base.Network.Proxy.Proxy | |||
import Standard.Base.Runtime.Context | |||
from Standard.Base.Network.HTTP import resolve_headers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is only executed if:
## To run this test locally:
$ sbt 'simple-httpbin/run localhost 8080'
$ export ENSO_HTTP_TEST_HTTPBIN_URL=http://localhost:8080/
Why don't we move the code from simple-httpbin
into enso-test-java-helpers and let this Http_Spec
start the server on its own?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds like a really cool idea indeed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll break this out as a separate task.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JaroslavTulach The purpose of this was not to improve the test infrastructure, but to provide better testing of |
@JaroslavTulach Renamed |
} | ||
} | ||
|
||
private String parsseTextEncoding(String contentType) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private String parsseTextEncoding(String contentType) { | |
private String parseTextEncoding(String contentType) { |
Pull Request Description
Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
./run ide build
.