Adding some polls API endpoints for PR to discourse_api, which work fine. Now I’m trying to understand how to create tests before submitting the PR, e.g.:
require 'spec_helper'
describe DiscourseApi::API::Polls do
subject { DiscourseApi::Client.new("http://localhost:3000", "test_d7fd0429940", "test_user" )}
describe "#polls" do
before do
stub_get("http://localhost:3000/polls/voters.json").to_return(body: fixture("voters.json"), headers: { content_type: "application/json" })
end
it "requests the correct resource" do
subject.voters :post_id => 27885, :poll_name => 'poll'
expect(a_get("http://localhost:3000/polls/voters.json")).to have_been_made
end
end
end
But am getting the error:
Failed to open TCP connection to localhost:3000 (Connection refused - connect(2) for "localhost" port 3000)
# ./lib/discourse_api/client.rb:141:in `rescue in request'
# ./lib/discourse_api/client.rb:132:in `request'
# ./lib/discourse_api/client.rb:85:in `get'
# ./lib/discourse_api/api/polls.rb:22:in `voters'
# ./spec/discourse_api/api/polls_spec.rb:12:in `block (3 levels) in <top (required)>'
# ------------------
# --- Caused by: ---
# Errno::ECONNREFUSED:
# Connection refused - connect(2) for "localhost" port 3000
# /Users/kimardenmiller/.rbenv/versions/2.5.3/gemsets/d_api/gems/webmock-2.3.2/lib/webmock/http_lib_adapters/net_http.rb:109:in `request'
All the existing tests pass fine for me, and I’m trying to copy the format of those existing tests, but cannot for the life of me figure out what I’m doing wrong.
Here’s the (new) endpoint being tested:
module DiscourseApi
module API
module Polls
def poll_voters(args)
args = API.params(args) # post_id, poll_name, user, opts = {}
.required(:post_id, :poll_name)
.optional(:opts, :api_username)
response = get("/polls/voters.json", args)
response[:body]
end
end
end
end
Have working tests, though my gut tells me I could use tips on making those urls more elegant.
require 'spec_helper'
describe DiscourseApi::API::Polls do
subject { DiscourseApi::Client.new("http://localhost:3000", "test_d7fd0429940", "test_user" )}
describe "#polls" do
before do
stub_get("http://localhost:3000/polls/voters.json?post_id=27885&poll_name=poll").to_return(body: fixture("polls_voters.json"), headers: { content_type: "application/json" })
end
it "requests the correct resource" do
subject.poll_voters post_id: 27885, poll_name: 'poll'
expect(a_get("http://localhost:3000/polls/voters.json?post_id=27885&poll_name=poll")).to have_been_made
end
it "returns the expected votes" do
voters = subject.poll_voters post_id: 27885, poll_name: 'poll'
expect(voters).to be_a Hash
voters.each { |g| expect(g).to be_an Array }
expect(voters['voters']['e539a9df8700d0d05c69356a07b768cf']).to be_an Array
expect(voters['voters']['e539a9df8700d0d05c69356a07b768cf'][0]['id']).to eq(356)
end
end
end
Glad you got the tests working. They can be tricky sometimes because if the request doesn’t match exactly it will try and create an actual request rather that using the stubbed request.
I think the urls are okay. It would be nice not to use such a high post_id, but I’m not concerned about that. And ALL the tests need to have http://localhost:3000 extracted out into a common variable, but that should probably be done in a separate commit. Whenever you are ready you can go ahead and submit a pr and I can review it.
One thing I did notice though is I that passing in the api_username as a parameter is no longer supported.
This is because the discourse_api gem now only passes in the auth via the headers, and discourse core ignores any auth credentials in the body if the header is already being used for auth.
Let’s split up the PRs please. First let’s focus on the spec one and let’s move the common variable into the spec folder. We should be able to add it to the spec helper file.
Because this is only for examples, we should have anything related to this inside of the examples directory and not have it edit any of the existing functionality inside of the lib directory. Even the config.yml file we should place inside of the examples directory. Loading the yaml file is pretty lightweight and we can add it to the top of each example file, but probably should add it to a common file inside of the examples directory they can all read from.
# host e.g. localhost:3000 or discourse.my_domain.com
host: YOUR_HOST_NAME
# api user (can effect results returned, e.g. .categories method returns only the categories your api_username can see)
# create a new client with when changing api user
api_username: YOUR_API_KEY
# api key from Discourse admin panel /admin/api/keys
api_key: YOUR_API_KEY
examples/badges.rb
require_relative 'example_helper'
# get badges
puts client.badges
Thoughts? Sure we need the yml now, given the examples_helper.rb?
Looks good. Let’s update the example helper with default values so that it will work without the yml file. Then update the readme with instructions to copy the example yml file to examples/config.yml. And be sure to add examples/config.yml to .gitignore.
This is a pretty standard convention so that it isn’t easy to commit a config.yml file with production credentials. And you can edit the file without git seeing it as a change.
I think that error is okay because this is how the examples currently work, and this is only in the examples anyways. If you want to, now or later, we can always catch the exception and provide a helpful message.
Our current Testing section is a little confusing to me, in that it mixes tasks related to discourse_api with installing Discourse itself. e.g. if you have a staging server in the cloud, you don’t need to install Discourse locally at all. How do you feel about: