Building Tests for New discourse_api Endpoints

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
3 Likes

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
1 Like

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.

4 Likes

Continuing the conversation from

Goals:

  • For Tests: Extract http://localhost:3000 out into a common variable

  • For Examples: Extract host, username, and api_key into a config.yml file

We are deciding where to place and load things. Right now I have in the discourse_api.rb file (w/config.yml in root dir):

require 'yaml'
CONFIG = YAML.load_file(File.expand_path('../../config.yml', __FILE__))

… then in each example file:

client = DiscourseApi::Client.new(CONFIG['host'])
client.api_key = CONFIG['api_key']
client.api_username = CONFIG['api_username']

That works for the examples, but not for the tests. I’m interested in some guidance on best practices for this PR.

1 Like

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.

3 Likes

@blake let me know if you have any comments on the design above.

This is fine.

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.

1 Like

Here’s what I have:

examples/example_helper.rb

$LOAD_PATH.unshift File.expand_path('../../lib', __FILE__)
require File.expand_path('../../lib/discourse_api', __FILE__)
require 'yaml'

CONFIG = YAML.load_file(File.expand_path('../../config.yml', __FILE__))

def client
  discourse_client = DiscourseApi::Client.new(CONFIG['host'])
  discourse_client.api_key = CONFIG['api_key']
  discourse_client.api_username = CONFIG['api_username']
  discourse_client
end

examples/config-example.yml

# 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?

1 Like

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.

1 Like

All good, except I don’t quite follow this one

example_helper.rb works as is once config.yml exists. We want it to also work without a yml file, but we still have a yml file?

1 Like

Yes, this way ruby doesn’t explode if they haven’t setup their yaml file yet.

Not exactly this, but something like:

  discourse_client = DiscourseApi::Client.new(CONFIG['host'] || 'localhost:3000')
  discourse_client.api_key = CONFIG['api_key'] || 'api-key'
  discourse_client.api_username = CONFIG['api_username'] || 'api-username'
1 Like

So, before the config.yml file is created, even with the double pipe logic we explode with

`initialize': No such file or directory @ rb_sysopen

For the pipes to even come into play we’d need to adjust the YAML.load_file as well.

Yes, we need to adjust that as well.

So this

config_yml = File.expand_path('../config.yml', __FILE__)

if File.exists? config_yml
  CONFIG = YAML.load_file config_yml
else
  CONFIG = {}
end

def client
  discourse_client = DiscourseApi::Client.new(CONFIG['host'] || 'http://localhost:3000')
  discourse_client.api_key = CONFIG['api_key'] || 'YOUR_API_KEY'
  discourse_client.api_username = CONFIG['api_username'] || 'YOUR_USERNAME'
  discourse_client
end

gives us errors like

Failed to open TCP connection to localhost:3000 (Connection refused - connect(2) for "localhost" port 3000) (Faraday::ConnectionFailed)

Is that our preferred error?

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.

Maybe …

config_yml = File.expand_path('../configg.yml', __FILE__)

begin
  CONFIG = YAML.load_file config_yml
rescue Errno::ENOENT
  raise ArgumentError, '/examples/config.yml file not found. Please copy config-example.yml to create a config.yml for your environment.'
end

def client
  discourse_client = DiscourseApi::Client.new(CONFIG['host'] || 'http://localhost:3000')
  discourse_client.api_key = CONFIG['api_key'] || 'YOUR_API_KEY'
  discourse_client.api_username = CONFIG['api_username'] || 'YOUR_USERNAME'
  discourse_client
end

Do we still need the pipes then? Leave them in?

1 Like

Yea that works. In that case we won’t need the pipes so yes you can remove them.

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:

Examples and Testing

To try the examples or run tests you will need a Discourse instance up and running, installed either locally or in the cloud.

Examples

To run the examples in /examples:

  1. Specify your environment by making a copy of config-example.yml to create a file called config.yml with your environment settings.
  2. In a given example file, comment out all but the examples you want to run.
  3. Edit the example file with any required parameters, e.g. username
  4. Running the file will then pull from your config.yml to run a given client method (e.g. client.badges) against your Discourse instance

Testing

To run the discourse_api tests in spec:

  1. Install bundler in the discourse_api directory, run gem install bundler
  2. Inside of your discourse_api directory, run: bundle exec rspec spec/

Sounds good, the readme could use some updating

To run the tests you don’t actually need a discourse instance up and running. All the spec requests should be mocked and a server isn’t needed.

Ah, right, so how about …

Examples

To try the examples in /examples you will need a Discourse instance up and running, installed either locally or in the cloud, then:

  1. Specify your environment by making a copy of config-example.yml to create a file called config.yml with your environment settings.
  2. In a given example file, comment out all but the examples you want to run.
  3. Edit the example file with any required parameters, e.g. a target username passed to the method.
  4. Running the file will then pull from your config.yml to run a given client method (e.g. client.badges) against your Discourse instance

Testing

To run the discourse_api tests in spec:

  1. Install bundler in the discourse_api directory, run gem install bundler
  2. Inside of your discourse_api directory, run: bundle exec rspec spec/
3 Likes