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

Actually mounts and curls an API #1927

Closed
wants to merge 2 commits into from
Closed

Conversation

myxoh
Copy link
Member

@myxoh myxoh commented Oct 30, 2019

This is a way of doing and end-end test of grape with a sample API and actually performing API calls against it.

I am open to suggestions.

Tests the issue described in: #1901

@grape-bot
Copy link

grape-bot commented Oct 30, 2019

1 Warning
⚠️ Unless you’re refactoring existing code, please update CHANGELOG.md.
1 Message
📖 We really appreciate pull requests that demonstrate issues, even without a fix. That said, the next step is to try and fix the failing tests!

Here's an example of a CHANGELOG.md entry:

* [#1927](https://github.com/ruby-grape/grape/pull/1927): Actually mounts and curls an api - [@myxoh](https://github.com/myxoh).

Generated by 🚫 danger

@dblock
Copy link
Member

dblock commented Oct 30, 2019

I like it! Would make sure those command line errors are handled, I believe system doesn't properly error out, so you want to be checking for exit code.

@myxoh
Copy link
Member Author

myxoh commented Nov 14, 2019

I've got rid of the logs. Not sure whether there was something else you were thinking about. Were you thinking about sig trapping?

  Signal.trap("TERM") do
    shutdown()
  end

let!(:server_thread) do
Thread.new do
`fuser -k 9292/tcp 2>/dev/null`
system("rackup spec/integration/real_api/example_api/config.ru 2>/dev/null")
Copy link
Member

Choose a reason for hiding this comment

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

I meant that this call will fail without an exception if, for example, rackup cannot run. You need to handle the return from it.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

@dblock
Copy link
Member

dblock commented Dec 4, 2019

Rebase/fixup?

@myxoh
Copy link
Member Author

myxoh commented Dec 4, 2019

Right. I need to spend some time getting this right, but it's been a very busy month.
Let me close it for now and get back to it

@myxoh myxoh closed this Dec 4, 2019
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.

None yet

3 participants