Skip to content

Commit

Permalink
Added comments describing the reasons that tests fail.
Browse files Browse the repository at this point in the history
  • Loading branch information
raisin committed Nov 4, 2016
1 parent 9cb391b commit 41c1127
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 0 deletions.
3 changes: 3 additions & 0 deletions spec/controllers/apipies_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
end

it "returns not_found on wrong method" do
# This spec fails because a get url parameter cannot be named method.
get :index, :version => "2.0", :resource => "architectures", :method => "wrong_method"

assert_response :not_found
Expand Down Expand Up @@ -225,6 +226,8 @@
expect(response.body).to eq("apidoc.json cache")
get :index, :version => 'v1', :format => "html", :resource => "resource"
expect(response.body).to eq("resource.html cache")
# get parameters cannot have the name method. Rails was built in such a way that this is a
# reserved keyword. As a result, this get request fails.

This comment has been minimized.

Copy link
@KjellMorgenstern

KjellMorgenstern Dec 21, 2016

I added a workaround in (see https://github.com/KjellMorgenstern/apipie-rails/tree/rails-5),
which uses params: { method: "the_method" } instead of directly passing :method.

I would collect and start a pull request, but this only makes sense when switching to rails 5. Do you know if there are currently any activities to bring apipie to rails 5?

This comment has been minimized.

Copy link
@raisin

raisin Dec 23, 2016

Author Owner

Hi KjellMorgenstern,

Happy to see your comments and contribution. Some comments:
1.) Not sure how you found this, but I did leave some comments at the following location linking to this branch in my fork.
Apipie#495
Perhaps it makes sense to add comments and links there? Of course, we need to rebase that branch against the current master.
2.) I wrote my code in a way that maintains backwards compatibility with earlier versions of Rails, but that makes the code more complex and more difficult to maintain. It's really upto the maintainers whether they want one version that handles all versions of Rails or a fork to handle Rails 5.x+ that is not backwards compatible.
3.) I don't know whether an effort is under way to create a version that is compatible with Rails 5. You are the first person that has contacted me since I posted on Nov. 3rd. Personally, I've been using my fork. It works with the caveat that api! fails. Furthermore, the use of api! causes the documentation to break. And, I haven't tested documentation generation from specs.
4.) Since no one responded to my post, I didn't put any additional time into this. I'm open to collaborating on a pull request if you'd like to work on this together.

Cheers,
raisin

get :index, :version => 'v1', :format => "html", :resource => "resource", :method => "method"
expect(response.body).to eq("method.html cache")
end
Expand Down
3 changes: 3 additions & 0 deletions spec/controllers/concerns_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
end

it "replaces a placeholder doc specified in concern with a real path" do
# This spec fails due to the failure of the api! dsl method.
# See the controller for the replacement line (without api!)
# that works.
path = Apipie["concern_resources#index"].apis.first.path
expect(path).to eq('/api/concerns')

Expand Down
4 changes: 4 additions & 0 deletions spec/controllers/users_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,10 @@ def reload_controllers
context "Using routes.rb" do
it "should contain basic info about method" do
a = Apipie[UsersController, :create_route]
# the spec below fails due to the fact that the api!
# dsl syntax is used. This portion of the dsl does not
# seem to be currently compatible with Rails 5.x.x.x
# routing.
expect(a.apis.count).to eq(1)
expect(a.formats).to eq(['json'])
api = a.apis.first
Expand Down
4 changes: 4 additions & 0 deletions spec/dummy/app/controllers/concerns/sample_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ module Concerns
module SampleController
extend Apipie::DSL::Concern

# Spec fails due to failure of the api! method.
# spec/controllers/concerns_controller_spec.rb:24 would pass if api!
# were replaced with the following line.
# api :GET, '/api/concerns'
api!
def index
render :text => "OK #{params.inspect}"
Expand Down
4 changes: 4 additions & 0 deletions spec/dummy/app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,10 @@ def desc_from_file
render :text => 'document from file action'
end

# Spec spec/controllers/users_controller_spec.rb:396
# fails due to use of api!. The spec would get further and almost pass
# if it was replaced with the following.
# api :POST, "/api/users/create_route", "Create user"
api! 'Create user using create_route'
param_group :user
param :user, Hash do
Expand Down

0 comments on commit 41c1127

Please sign in to comment.