Numbers: random numbers facts #1
Conversation
There was a problem hiding this comment.
Let's use this simple case to decide on two important things:
- How should they be tested? At least some "sanity tests" are necessary (e.g. that it really successfully calls API not fails into 404 or something)
- How should they be documented? So
Tlawsuser can look somewhere for quick-reference for the API wrapper.
lib/tlaws/numbers.rb
Outdated
| Facts about numbers. | ||
| } | ||
|
|
||
| param :numb, required: true |
There was a problem hiding this comment.
Let's keep it number, as in original docs. Two chars is not that much economy :)
lib/tlaws/numbers.rb
Outdated
| desc %Q{ | ||
| Get an interesting fact about the random number. | ||
| } | ||
| end |
There was a problem hiding this comment.
I believe the same endpoints repeats for one number ([]), and for the random number? Then let's make it DRY somehow (var.1: just do something like %i[random, []].each do |namespace| ..., var.2: try to introduce some solution in TLAW itself, that will allow to describe "shared endpoints" (like "shared examples" in RSpec), and then just include_endpoints "number endpoints" in DSL?)
|
@zverok, made some changes and added tests. Please review it. |
| @@ -3,3 +3,5 @@ | |||
| module Tlaws | |||
| # Your code goes here... | |||
| end | |||
|
|
|||
| require_relative 'tlaws/numbers' | |||
There was a problem hiding this comment.
I don't think we should require them all here. Imagine some of the wrappers depend on other libraries (like nokogiri, or some geo-services, or even SQL databases) — we don't want all of it to be required at once. Typical use case is "I install entire Tlaws (say, 30 wrappers), but use only wrappers X and Y, so I require only them".
|
|
||
| end_path = '?json' | ||
|
|
||
| if ns == :[] |
There was a problem hiding this comment.
ifs look bad in declarative API description. TLAW supports (or intended to support, at least, check it!) continuing of descriptions in several blocks, so it should be written like this:
namespace :[], '/numbers' do
desc ...
param ...
end
namespace :random do
desc ...
param ...
end
%i[[], random].each do |ns|
namespace ns do
...other code that is common for both of them
end
end| Can be a: | ||
| * singe number - numbers_api[42].{type} | ||
| * range of numbers - numbers_api[42..56].{type} | ||
| * combined array - numbers_api[[42..56, 77, 285, 770..777]].{type} |
There was a problem hiding this comment.
TLAW should support splat arguments (e.g. [42..56, 77, 285, 770..777] without second brackets), but currently it does not. Please add GitHub issue in TLAW repo, and implement it, or ask me to :)
| } | ||
| param :max, keyword: false, default: nil, desc: %Q{ | ||
| Maximum number for random | ||
| } |
There was a problem hiding this comment.
Non-keyword arguments don't seem like a good solution to me. random(10) intuitively reads like "random 0..10", not "random 10..Infinity". And the most useful call sequence ("give me random number less than some boundary") looks the most ugly: random(nil, 10)
| param :max, keyword: false, default: nil, desc: %Q{ | ||
| Maximum number for random | ||
| } | ||
| end_path += '&min={min}&max={max}' |
| end_path += '&min={min}&max={max}' | ||
| end | ||
|
|
||
| endpoint :math, '/math' + end_path do |
There was a problem hiding this comment.
Please post TLAW GitHub issue for default always present GET argument.
| it "does something useful" do | ||
| expect(false).to eq(true) | ||
| end | ||
| let(:numbersapi){ Tlaws::Numbers.new } |
There was a problem hiding this comment.
All of it should go to spec/tlaws/numbers_spec.rb
|
|
||
| describe '.random' do | ||
|
|
||
| it "get an answer" do |
There was a problem hiding this comment.
Something bad goes on with indentation here :) I strongly recommend add rubocop to the repo on the earliest stages ;)
| end | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
About testing strategy in total:
- I believe that at least on early stages it should be as short "sanity checks" as possible, our primary goal is to not get bored with it, make sure it works.
- Make it as readable and writable as possible, for easier update.
- Make it fast.
So, the following seems to be a better strategy for this case (though not usual for RSpec in general):
describe Tlaws::Numbers do
let(:api) { described_class.new }
it 'works' do
expect(api[20].year).to include(something something)
expect(api[20].day).to include(something something)
expect(api.random(max: 3)).to ...
expect(api.random(min: 200)).to ...
end
endWDYT?
...and add VCR, of course ;)
Prototype numbersapi