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

Add barebones asynchttpserver tests #13883

Merged
merged 2 commits into from
Apr 5, 2020
Merged

Add barebones asynchttpserver tests #13883

merged 2 commits into from
Apr 5, 2020

Conversation

supakeen
Copy link
Contributor

@supakeen supakeen commented Apr 5, 2020

The regression in #13866 showed that there were no tests for asynchttpserver so I took a stab at some barebones tests which at least try to:

  • Perform a request
  • Perform a request with a different status code
  • Perform a request with a custom headers response.
  • Perform a request with a custom content-length header in the response.

While the request proc in the tests could be simplified with another template I think future test cases might want to perform multiple requests.

Since this is my first 'serious' Nim code aside from the PR for the abovementioned ticket (a one line change) I'd like some help getting it up to par.

@@ -0,0 +1,119 @@
discard """
cmd: "nim c --threads:on $file"
Copy link

@ghost ghost Apr 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is threads:on needed here though? I don't see you using threads anywhere in the file

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still it's a good idea.

@Araq
Copy link
Member

Araq commented Apr 5, 2020

I like it. We'll later see if it's a "flaky" test or not.

@Araq Araq merged commit 8784715 into nim-lang:devel Apr 5, 2020
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