Création de tests pour les nouveaux endpoints discourse_api

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.

Suite à la conversation sur

Objectifs :

  • Pour les tests : Extraire http://localhost:3000 dans une variable commune

  • Pour les exemples : Extraire l’hôte, le nom d’utilisateur et la clé API dans un fichier config.yml

Nous décidons où placer et charger les éléments. Pour l’instant, j’ai ceci dans le fichier discourse_api.rb (avec config.yml dans le répertoire racine) :

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

… puis dans chaque fichier d’exemple :

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

Cela fonctionne pour les exemples, mais pas pour les tests. Je suis intéressé par des conseils sur les meilleures pratiques pour cette PR.

Séparons les PRs, s’il vous plaît. Concentrons-nous d’abord sur celle relative à la spécification et déplaçons la variable commune dans le dossier spec. Nous devrions pouvoir l’ajouter au fichier d’aide de test (spec helper).

@blake, fais-moi savoir si tu as des commentaires sur la conception ci-dessus.

Cela convient.

Comme il ne s’agit que d’exemples, tout ce qui y est lié doit se trouver dans le répertoire examples et ne doit modifier aucune des fonctionnalités existantes du répertoire lib. Même le fichier config.yml devrait être placé dans le répertoire examples. Le chargement du fichier YAML est très léger ; nous pouvons l’ajouter en haut de chaque fichier d’exemple, mais il serait préférable de le placer dans un fichier commun au sein du répertoire examples, auquel tous pourront accéder.

Voici ce que j’ai :

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 par exemple localhost:3000 ou discourse.mon_domaine.com

host: VOTRE_NOM_DE_HOST

utilisateur de l’API (peut affecter les résultats retournés, par exemple la méthode .categories ne renvoie que les catégories visibles par votre api_username)

créez un nouveau client lors du changement d’utilisateur de l’API

api_username: VOTRE_API_KEY

clé API depuis le panneau d’administration de Discourse /admin/api/keys

api_key: VOTRE_API_KEY

examples/badges.rb

require_relative ‘example_helper’

obtenir les badges

puts client.badges

Qu’en pensez-vous ? Sommes-nous vraiment obligés d’avoir le fichier yml maintenant, étant donné l’exemple_helper.rb ?

Ça a l’air bien. Mettons à jour le helper d’exemple avec des valeurs par défaut afin qu’il fonctionne sans le fichier yml. Ensuite, mettez à jour le fichier README avec les instructions pour copier le fichier yml d’exemple vers examples/config.yml. Et assurez-vous d’ajouter examples/config.yml au fichier .gitignore.

C’est une convention assez courante pour éviter de commettre facilement un fichier config.yml contenant des identifiants de production. Vous pourrez ainsi modifier le fichier sans que Git le signale comme une modification.

Tout va bien, sauf que je ne comprends pas tout à fait celui-ci

example_helper.rb fonctionne tel quel une fois que config.yml existe. Nous voulons qu’il fonctionne également sans fichier yml, mais nous avons toujours un fichier yml ?

Oui, ainsi Ruby ne plante pas s’ils n’ont pas encore configuré leur fichier YAML.

Pas exactement cela, mais quelque chose comme :

  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'

Donc, avant que le fichier config.yml ne soit créé, même avec la logique des doubles pipes, nous obtenons une erreur :

`initialize' : Aucun fichier ou répertoire du type @ rb_sysopen

Pour que les pipes entrent même en jeu, il faudrait également ajuster YAML.load_file.

Oui, il faut aussi l’ajuster.

Donc ce code

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

nous renvoie des erreurs comme

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

Est-ce l’erreur que nous souhaitons obtenir ?

Je pense que cette erreur est acceptable car c’est ainsi que fonctionnent actuellement les exemples, et de toute façon, cela ne concerne que les exemples. Si vous le souhaitez, maintenant ou plus tard, nous pouvons toujours capturer l’exception et fournir un message utile.

Peut-être …

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

begin
  CONFIG = YAML.load_file config_yml
rescue Errno::ENOENT
  raise ArgumentError, 'Fichier /examples/config.yml introuvable. Veuillez copier config-example.yml pour créer un fichier config.yml adapté à votre environnement.'
end

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

Faut-il toujours conserver les barres verticales ? Les laisser ?

Oui, ça marche. Dans ce cas, nous n’aurons pas besoin des tuyaux, donc oui, tu peux les supprimer.

Notre section Tests actuelle me paraît un peu confuse, car elle mélange des tâches liées à discourse_api avec l’installation de Discourse lui-même. Par exemple, si vous avez un serveur de préproduction dans le cloud, vous n’avez pas besoin d’installer Discourse localement. Qu’en pensez-vous :

Exemples et Tests

Pour essayer les exemples ou exécuter les tests, vous aurez besoin d’une instance Discourse opérationnelle, installée soit localement, soit dans le cloud.

Exemples

Pour exécuter les exemples situés dans /examples :

  1. Spécifiez votre environnement en copiant config-example.yml pour créer un fichier nommé config.yml contenant vos paramètres d’environnement.
  2. Dans un fichier d’exemple donné, commentez tout sauf les exemples que vous souhaitez exécuter.
  3. Modifiez le fichier d’exemple avec les paramètres requis, par exemple username.
  4. L’exécution du fichier utilisera alors votre config.yml pour appeler une méthode client donnée (par exemple client.badges) sur votre instance Discourse.

Tests

Pour exécuter les tests de discourse_api situés dans spec :

  1. Installez bundler dans le répertoire discourse_api en exécutant gem install bundler.
  2. Dans votre répertoire discourse_api, exécutez : bundle exec rspec spec/.

Ça semble bon, le fichier README pourrait être mis à jour.

Pour exécuter les tests, vous n’avez pas réellement besoin d’une instance Discourse en cours d’exécution. Toutes les requêtes de spécification doivent être simulées et un serveur n’est pas nécessaire.

Ah, d’accord, alors qu’en pensez-vous…

Exemples

Pour essayer les exemples situés dans /examples, vous aurez besoin d’une instance Discourse en cours d’exécution, installée soit localement, soit dans le cloud, puis :

  1. Spécifiez votre environnement en copiant le fichier config-example.yml pour créer un fichier nommé config.yml contenant vos paramètres d’environnement.
  2. Dans un fichier d’exemple donné, commentez tout sauf les exemples que vous souhaitez exécuter.
  3. Modifiez le fichier d’exemple avec les paramètres requis, par exemple un username cible passé à la méthode.
  4. L’exécution du fichier utilisera alors votre config.yml pour exécuter une méthode client donnée (par exemple client.badges) sur votre instance Discourse.

Tests

Pour exécuter les tests de discourse_api dans spec :

  1. Installez bundler dans le répertoire discourse_api en exécutant gem install bundler.
  2. Dans votre répertoire discourse_api, exécutez : bundle exec rspec spec/.