You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by hdiedrich <gi...@git.apache.org> on 2014/11/28 18:08:24 UTC
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
GitHub user hdiedrich opened a pull request:
https://github.com/apache/couchdb-chttpd/pull/10
Dynamic endpoints handlers
chttpd hardcoded handlers are replaced with with dynamic url handlers,
which are functions in a dynamically created module, assembled from
priv/chttpd_handler.cfg files from all interested applications.
For couched these are currently: chttpd, mem3 and global_changes.
This is a special branch that pull the prepared branches (of the same name)
of chttpd, mem3 and global_changes, which have the config files.
Run 'make check' to test the handlers. They are implicitly tested by other
tests and there is a whitebox callback test chttpd_handler_callback_test,
which calls the dynamic functions directly and mocks the funs that they
return. This tests precisely the relationship between the *_handler function
clauses and the returned function.
There are more tests to come, both testing the endpoints via http, and the
configuration assembly.
BugzID: 27037
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/hdiedrich/couchdb-chttpd 27037-5-dynamic-endpoint-handlers
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-chttpd/pull/10.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #10
----
commit 2fe0c7e1c1b701e8aee7631ad548085c9eaac55b
Author: H. Diedrich <hd...@eonblast.com>
Date: 2014-11-28T15:08:12Z
Dynamic endpoints handlers
chttpd hardcoded handlers are replaced with with dynamic url handlers,
which are functions in a dynamically created module, assembled from
priv/chttpd_handler.cfg files from all interested applications.
For couched these are currently: chttpd, mem3 and global_changes.
This is a special branch that pull the prepared branches (of the same name)
of chttpd, mem3 and global_changes, which have the config files.
Run 'make check' to test the handlers. They are implicitly tested by other
tests and there is a whitebox callback test chttpd_handler_callback_test,
which calls the dynamic functions directly and mocks the funs that they
return. This tests precisely the relationship between the *_handler function
clauses and the returned function.
There are more tests to come, both testing the endpoints via http, and the
configuration assembly.
BugzID: 27037
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#discussion_r21087260
--- Diff: test/chttpd_handler_callback_test.erl ---
@@ -0,0 +1,266 @@
+%% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+%% use this file except in compliance with the License. You may obtain a copy of
+%% the License at
+%%
+%% http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+%% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+%% License for the specific language governing permissions and limitations under
+%% the License.
+%%
+%% @doc
+%% These whitebox tests call the endpoints that are implemented by dynamic funs,
+%% which replace the previously hardwired handlers, with a dynamically created
+%% module. This test mocks everything except the proper relationship between
+%% handler function clause and returned fun. The handler functions are called
+%% directly.
+%%
+%% This can look like mocks testing themselves. In this case, in other words:
+%% think of the former url_handler function in chttpd.erl: The relationship
+%% between a function clause and the specific returned fun() is exactly what is
+%% established by the dynamically created module. And it's precisely what this
+%% test verifies, for every endpoint.
+%%
+%% This test was part of chttpd-cloudant dbcore-2.7.7, verifying the original
+%% hardcoded endpoints. It has since been reworked but remains functionally
+%% mostly identical; the config source and processing has been replaced.
+
+-module(chttpd_handler_callback_test).
+
+-include_lib("eunit/include/eunit.hrl").
+
+all_test_() ->
+ io:format(user, "~nEndpoint handler callbacks:~n", []),
+ {setup,
+ fun() -> chttpd_handler:build(test_cfg()) end,
+ [
--- End diff --
Yes, something like that. Anonymous functions aren't good in error tracebacks nor in reports.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#issuecomment-69365389
I'm happy with merge this. @rnewson @davisp @chewbranca what do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by hdiedrich <gi...@git.apache.org>.
Github user hdiedrich commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#discussion_r21086072
--- Diff: test/chttpd_handler_reload_test.erl ---
@@ -0,0 +1,111 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(chttpd_handler_reload_test).
+
+-compile(export_all).
+
+-include_lib("eunit/include/eunit.hrl").
+
+%% for testing
+
+a() -> a.
+b() -> b.
+c() -> c.
+d() -> d.
+e() -> e.
+f() -> f.
+g() -> g.
+h() -> h.
+
+-spec reload_test() -> ok.
+%% @doc verifies that there are no side effects from prior builds.
+reload_test() ->
--- End diff --
Not sure you are taking the test for what it is. I added a comment explaining the module. It tests specifically that there are no traces left of a prior configuration, which I thought is not global.
However I do have a way more global test coming that does stuff like etap, e.g. firing up a mini cluster to test the endpoints. I'll be happy to discuss, maybe on IRC?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#issuecomment-65078078
All cool, but seems your logging is still there. Try this trick:
```
diff --git a/test/chttpd_handler_callback_test.erl b/test/chttpd_handler_callback_test.erl
index d1f340d..2050106 100644
--- a/test/chttpd_handler_callback_test.erl
+++ b/test/chttpd_handler_callback_test.erl
@@ -31,179 +31,179 @@
-include_lib("eunit/include/eunit.hrl").
+
all_test_() ->
- io:format(user, "~nEndpoint handler callbacks:~n", []),
- {setup,
+ {foreach,
fun() -> chttpd_handler:build(test_cfg()) end,
% url handlers
- [fun test_empty_string/0,
- fun test_favicon/0,
- fun test_utils/0,
- fun test_all_dbs/0,
- fun test_active_tasks/0,
- fun test_config/0,
- fun test_reload_query_servers/0,
- fun test_replicate/0,
- fun test_uuids/0,
- fun test_sleep/0,
- fun test_session/0,
- fun test_oauth/0,
- fun test_up/0,
- fun test_membership/0,
- fun test_db_updates/0,
- fun test_anything/0,
+ [fun test_empty_string/1,
+ fun test_favicon/1,
+ fun test_utils/1,
+ fun test_all_dbs/1,
+ fun test_active_tasks/1,
+ fun test_config/1,
+ fun test_reload_query_servers/1,
+ fun test_replicate/1,
+ fun test_uuids/1,
+ fun test_sleep/1,
+ fun test_session/1,
+ fun test_oauth/1,
+ fun test_up/1,
+ fun test_membership/1,
+ fun test_db_updates/1,
+ fun test_anything/1,
% Test the tests: if the final target function is missing, the call must
% fail, with any parameter. All parameters are valid.
- fun verify_unmocked_failing_empty_string/0,
- fun verify_unmocked_failing_favicon/0,
- fun verify_unmocked_failing_anything/0,
-
+ fun verify_unmocked_failing_empty_string/1,
+ fun verify_unmocked_failing_favicon/1,
+ fun verify_unmocked_failing_anything/1,
+
% db url handler tests,
- fun test_view_cleanup/0,
- fun test_compact/0,
- fun test_design/0,
- fun test_temp_view/0,
- fun test_changes/0,
- fun test_shards/0,
-
+ fun test_view_cleanup/1,
+ fun test_compact/1,
+ fun test_design/1,
+ fun test_temp_view/1,
+ fun test_changes/1,
+ fun test_shards/1,
+
% Test the test: when the final target function is missing, the Fun call
% must fail, with any argument, including valid ones.
- fun verify_unmocked_failing_view_cleanup/0,
- fun verify_unmocked_db_failing_something/0,
+ fun verify_unmocked_failing_view_cleanup/1,
+ fun verify_unmocked_db_failing_something/1,
% design url handler tests
- fun test_view/0,
- fun test_show/0,
- fun test_list/0,
- fun test_update/0,
- fun test_info/0,
- fun test_rewrite/0,
+ fun test_view/1,
+ fun test_show/1,
+ fun test_list/1,
+ fun test_update/1,
+ fun test_info/1,
+ fun test_rewrite/1,
% Test the test: when the final target function is missing, the Fun call
% must fail with any argument, including valid ones.
- fun verify_unmocked_failing_view/0,
- fun verify_unmocked_design_failing_something/0]}.
+ fun verify_unmocked_failing_view/1,
+ fun verify_unmocked_design_failing_something/1]}.
-test_empty_string() ->
- assertReturns("", chttpd_misc, handle_welcome_req).
+test_empty_string(_Cfg) ->
+ ?_test(assertReturns("", chttpd_misc, handle_welcome_req)).
-test_favicon() ->
- assertReturns("favicon.ico", chttpd_misc, handle_favicon_req).
+test_favicon(_Cfg) ->
+ ?_test(assertReturns("favicon.ico", chttpd_misc, handle_favicon_req)).
-test_utils() ->
- assertReturns("_utils", chttpd_misc, handle_utils_dir_req).
+test_utils(_Cfg) ->
+ ?_test(assertReturns("_utils", chttpd_misc, handle_utils_dir_req)).
-test_all_dbs() ->
- assertReturns("_all_dbs", chttpd_misc, handle_all_dbs_req).
+test_all_dbs(_Cfg) ->
+ ?_test(assertReturns("_all_dbs", chttpd_misc, handle_all_dbs_req)).
-test_active_tasks() ->
- assertReturns("_active_tasks", chttpd_misc, handle_task_status_req).
+test_active_tasks(_Cfg) ->
+ ?_test(assertReturns("_active_tasks", chttpd_misc, handle_task_status_req)).
-test_config() ->
- assertReturns("_config", chttpd_misc, handle_config_req).
+test_config(_Cfg) ->
+ ?_test(assertReturns("_config", chttpd_misc, handle_config_req)).
-test_reload_query_servers() ->
- assertReturns("_reload_query_servers", chttpd_misc,
- handle_reload_query_servers_req).
+test_reload_query_servers(_Cfg) ->
+ ?_test(assertReturns("_reload_query_servers", chttpd_misc,
+ handle_reload_query_servers_req)).
-test_replicate() ->
- assertReturns("_replicate", chttpd_misc, handle_replicate_req).
+test_replicate(_Cfg) ->
+ ?_test(assertReturns("_replicate", chttpd_misc, handle_replicate_req)).
-test_uuids() ->
- assertReturns("_uuids", chttpd_misc, handle_uuids_req).
+test_uuids(_Cfg) ->
+ ?_test(assertReturns("_uuids", chttpd_misc, handle_uuids_req)).
-test_sleep() ->
- assertReturns("_sleep", chttpd_misc, handle_sleep_req).
+test_sleep(_Cfg) ->
+ ?_test(assertReturns("_sleep", chttpd_misc, handle_sleep_req)).
-test_session() ->
- assertReturns("_session", chttpd_auth, handle_session_req).
+test_session(_Cfg) ->
+ ?_test(assertReturns("_session", chttpd_auth, handle_session_req)).
-test_oauth() ->
- assertReturns("_oauth", couch_httpd_oauth, handle_oauth_req).
+test_oauth(_Cfg) ->
+ ?_test(assertReturns("_oauth", couch_httpd_oauth, handle_oauth_req)).
-test_up() ->
- assertReturns("_up", chttpd_misc, handle_up_req).
+test_up(_Cfg) ->
+ ?_test(assertReturns("_up", chttpd_misc, handle_up_req)).
-test_membership() ->
- assertReturns("_membership", mem3_httpd, handle_membership_req).
+test_membership(_Cfg) ->
+ ?_test(assertReturns("_membership", mem3_httpd, handle_membership_req)).
-test_db_updates() ->
- assertReturns("_db_updates", global_changes_httpd,
- handle_global_changes_req).
+test_db_updates(_Cfg) ->
+ ?_test(assertReturns("_db_updates", global_changes_httpd,
+ handle_global_changes_req)).
-test_anything() ->
- assertReturns("anything", chttpd_db, handle_request).
+test_anything(_Cfg) ->
+ ?_test(assertReturns("anything", chttpd_db, handle_request)).
-verify_unmocked_failing_empty_string() ->
- assertUnmockedFails("", chttpd_misc).
+verify_unmocked_failing_empty_string(_Cfg) ->
+ ?_test(assertUnmockedFails("", chttpd_misc)).
-verify_unmocked_failing_favicon() ->
- assertUnmockedFails("favicon.ico", chttpd_misc).
+verify_unmocked_failing_favicon(_Cfg) ->
+ ?_test(assertUnmockedFails("favicon.ico", chttpd_misc)).
-verify_unmocked_failing_anything() ->
- assertUnmockedFails(anything, chttpd_db).
+verify_unmocked_failing_anything(_Cfg) ->
+ ?_test(assertUnmockedFails(anything, chttpd_db)).
-test_view_cleanup() ->
- assertReturns(db_url_handlers, <<"_view_cleanup">>, chttpd_db,
- handle_view_cleanup_req, 2).
+test_view_cleanup(_Cfg) ->
+ ?_test(assertReturns(db_url_handlers, <<"_view_cleanup">>, chttpd_db,
+ handle_view_cleanup_req, 2)).
-test_compact() ->
- assertReturns(db_url_handlers, <<"_compact">>, chttpd_db,
- handle_compact_req, 2).
+test_compact(_Cfg) ->
+ ?_test(assertReturns(db_url_handlers, <<"_compact">>, chttpd_db,
+ handle_compact_req, 2)).
-test_design() ->
- assertReturns(db_url_handlers, <<"_design">>, chttpd_db,
- handle_design_req, 2).
+test_design(_Cfg) ->
+ ?_test(assertReturns(db_url_handlers, <<"_design">>, chttpd_db,
+ handle_design_req, 2)).
-test_temp_view() ->
- assertReturns(db_url_handlers, <<"_temp_view">>, chttpd_view,
- handle_temp_view_req, 2).
+test_temp_view(_Cfg) ->
+ ?_test(assertReturns(db_url_handlers, <<"_temp_view">>, chttpd_view,
+ handle_temp_view_req, 2)).
-test_changes() ->
- assertReturns(db_url_handlers, <<"_changes">>, chttpd_db,
- handle_changes_req, 2).
+test_changes(_Cfg) ->
+ ?_test(assertReturns(db_url_handlers, <<"_changes">>, chttpd_db,
+ handle_changes_req, 2)).
-test_shards() ->
- assertReturns(db_url_handlers, <<"_shards">>, mem3_httpd,
- handle_shards_req, 2).
+test_shards(_Cfg) ->
+ ?_test(assertReturns(db_url_handlers, <<"_shards">>, mem3_httpd,
+ handle_shards_req, 2)).
-verify_unmocked_failing_view_cleanup() ->
- assertUnmockedFails(db_url_handlers, <<"_view_cleanup">>, chttpd_db, 2).
+verify_unmocked_failing_view_cleanup(_Cfg) ->
+ ?_test(assertUnmockedFails(db_url_handlers, <<"_view_cleanup">>, chttpd_db, 2)).
-verify_unmocked_db_failing_something() ->
- assertUnknownFails(db_url_handlers, <<"_something">>).
+verify_unmocked_db_failing_something(_Cfg) ->
+ ?_test(assertUnknownFails(db_url_handlers, <<"_something">>)).
-test_view() ->
- assertReturns(design_url_handlers, <<"_view">>, chttpd_view,
- handle_view_req, 3).
+test_view(_Cfg) ->
+ ?_test(assertReturns(design_url_handlers, <<"_view">>, chttpd_view,
+ handle_view_req, 3)).
-test_show() ->
- assertReturns(design_url_handlers, <<"_show">>, chttpd_show,
- handle_doc_show_req, 3).
+test_show(_Cfg) ->
+ ?_test(assertReturns(design_url_handlers, <<"_show">>, chttpd_show,
+ handle_doc_show_req, 3)).
-test_list() ->
- assertReturns(design_url_handlers, <<"_list">>, chttpd_show,
- handle_view_list_req, 3).
+test_list(_Cfg) ->
+ ?_test(assertReturns(design_url_handlers, <<"_list">>, chttpd_show,
+ handle_view_list_req, 3)).
-test_update() ->
- assertReturns(design_url_handlers, <<"_update">>, chttpd_show,
- handle_doc_update_req, 3).
+test_update(_Cfg) ->
+ ?_test(assertReturns(design_url_handlers, <<"_update">>, chttpd_show,
+ handle_doc_update_req, 3)).
-test_info() ->
- assertReturns(design_url_handlers, <<"_info">>, chttpd_db,
- handle_design_info_req, 3).
+test_info(_Cfg) ->
+ ?_test(assertReturns(design_url_handlers, <<"_info">>, chttpd_db,
+ handle_design_info_req, 3)).
-test_rewrite() ->
- assertReturns(design_url_handlers, <<"_rewrite">>, chttpd_rewrite,
- handle_rewrite_req, 3).
+test_rewrite(_Cfg) ->
+ ?_test(assertReturns(design_url_handlers, <<"_rewrite">>, chttpd_rewrite,
+ handle_rewrite_req, 3)).
-verify_unmocked_failing_view() ->
- assertUnmockedFails(design_url_handlers, <<"_view">>, chttpd_view, 3).
+verify_unmocked_failing_view(_Cfg) ->
+ ?_test(assertUnmockedFails(design_url_handlers, <<"_view">>, chttpd_view, 3)).
-verify_unmocked_design_failing_something() ->
- assertUnknownFails(design_url_handlers, <<"_something">>).
+verify_unmocked_design_failing_something(_Cfg) ->
+ ?_test(assertUnknownFails(design_url_handlers, <<"_something">>)).
%% Call the dynamic function with a parameter known to trigger a specific
@@ -212,10 +212,9 @@ verify_unmocked_design_failing_something() ->
assertReturns(Endpoint, M, F) ->
meck:new(M, [passthrough, non_strict]),
try
- io:format(user, "~-47...s ", [Endpoint]),
meck:expect(M, F, fun(X) -> {return, Endpoint, X} end),
Fun = chttpd_handler:url_handler(Endpoint),
- ?_assertEqual({return, Endpoint, x}, Fun(x))
+ ?assertEqual({return, Endpoint, x}, Fun(x))
after
meck:unload(M)
end.
@@ -225,7 +224,7 @@ assertUnmockedFails(Endpoint, M) ->
meck:new(M, [non_strict]),
try
Fun = chttpd_handler:url_handler(Endpoint),
- ?_assertError(undef, Fun(x))
+ ?assertError(undef, Fun(x))
after
meck:unload(M)
end.
@@ -234,7 +233,6 @@ assertUnmockedFails(Endpoint, M) ->
assertReturns(HandlerLister, Endpoint, M, F, Arity) ->
meck:new(M, [passthrough, non_strict]),
try
- io:format(user, "~-47...s ", [Endpoint]),
case Arity of
2 ->
meck:expect(M, F, fun(X, Y) -> {return, Endpoint, X, Y} end),
@@ -243,7 +241,7 @@ assertReturns(HandlerLister, Endpoint, M, F, Arity) ->
3 ->
meck:expect(M, F, fun(X, Y, Z) -> {return, Endpoint, X, Y, Z} end),
{_, Fun} = lists:keyfind(Endpoint, 1, chttpd_handler:HandlerLister()),
- ?_assertEqual({return, Endpoint, x, y, z}, Fun(x, y, z))
+ ?assertEqual({return, Endpoint, x, y, z}, Fun(x, y, z))
end
after
meck:unload(M)
@@ -255,8 +253,8 @@ assertUnmockedFails(HandlerLister, Endpoint, M, Arity) ->
try
{_, Fun} = lists:keyfind(Endpoint, 1, chttpd_handler:HandlerLister()),
case Arity of
- 2 -> ?_assertError(undef, Fun(x, y));
- 3 -> ?_assertError(undef, Fun(x, y, z))
+ 2 -> ?assertError(undef, Fun(x, y));
+ 3 -> ?assertError(undef, Fun(x, y, z))
end
after
meck:unload(M)
@@ -264,7 +262,7 @@ assertUnmockedFails(HandlerLister, Endpoint, M, Arity) ->
%% Make sure that a wrong parameter also really fails.
assertUnknownFails(HandlerLister, Endpoint) ->
- false = lists:keyfind(Endpoint, 1, chttpd_handler:HandlerLister()).
+ ?assertEqual(false, lists:keyfind(Endpoint, 1, chttpd_handler:HandlerLister())).
test_cfg() ->
[{url_handler, clauses, [
```
And compare the output yours:
```
module 'chttpd_handler_callback_test'
chttpd_handler_callback_test: all_test_.................................................. [0.142 s] ok
chttpd_handler_callback_test: all_test_...favicon.ico.................................... [0.075 s] ok
chttpd_handler_callback_test: all_test_..._utils......................................... [0.068 s] ok
chttpd_handler_callback_test: all_test_..._all_dbs....................................... [0.069 s] ok
chttpd_handler_callback_test: all_test_..._active_tasks.................................. [0.071 s] ok
chttpd_handler_callback_test: all_test_..._config........................................ [0.070 s] ok
chttpd_handler_callback_test: all_test_..._reload_query_servers.......................... [0.069 s] ok
chttpd_handler_callback_test: all_test_..._replicate..................................... [0.068 s] ok
chttpd_handler_callback_test: all_test_..._uuids......................................... [0.079 s] ok
chttpd_handler_callback_test: all_test_..._sleep......................................... [0.092 s] ok
chttpd_handler_callback_test: all_test_..._session....................................... [0.007 s] ok
chttpd_handler_callback_test: all_test_..._oauth......................................... [0.075 s] ok
chttpd_handler_callback_test: all_test_..._up............................................ [0.069 s] ok
chttpd_handler_callback_test: all_test_..._membership.................................... [0.030 s] ok
chttpd_handler_callback_test: all_test_..._db_updates.................................... [0.042 s] ok
chttpd_handler_callback_test: all_test_...anything....................................... [0.284 s] ok
chttpd_handler_callback_test: all_test_...[0.061 s] ok
chttpd_handler_callback_test: all_test_...[0.064 s] ok
chttpd_handler_callback_test: all_test_...[0.290 s] ok
chttpd_handler_callback_test: all_test_..._view_cleanup.................................. [0.364 s] ok
chttpd_handler_callback_test: all_test_..._compact....................................... [0.322 s] ok
chttpd_handler_callback_test: all_test_..._design........................................ [0.320 s] ok
chttpd_handler_callback_test: all_test_..._temp_view..................................... [0.026 s] ok
chttpd_handler_callback_test: all_test_..._changes....................................... [0.296 s] ok
chttpd_handler_callback_test: all_test_..._shards........................................ [0.020 s] ok
chttpd_handler_callback_test: all_test_...[0.323 s] ok
chttpd_handler_callback_test: all_test_...ok
chttpd_handler_callback_test: all_test_..._view.......................................... [0.029 s] ok
chttpd_handler_callback_test: all_test_..._show.......................................... [0.080 s] ok
chttpd_handler_callback_test: all_test_..._list.......................................... [0.054 s] ok
chttpd_handler_callback_test: all_test_..._update........................................ [0.052 s] ok
chttpd_handler_callback_test: all_test_..._info.......................................... [0.351 s] ok
chttpd_handler_callback_test: all_test_..._rewrite....................................... [0.062 s] ok
chttpd_handler_callback_test: all_test_...[0.031 s] ok
chttpd_handler_callback_test: all_test_...ok
```
And now:
```
module 'chttpd_handler_callback_test'
chttpd_handler_callback_test:90: test_empty_string...[0.087 s] ok
chttpd_handler_callback_test:93: test_favicon...[0.076 s] ok
chttpd_handler_callback_test:96: test_utils...[0.076 s] ok
chttpd_handler_callback_test:99: test_all_dbs...[0.075 s] ok
chttpd_handler_callback_test:102: test_active_tasks...[0.077 s] ok
chttpd_handler_callback_test:105: test_config...[0.076 s] ok
chttpd_handler_callback_test:108: test_reload_query_servers...[0.077 s] ok
chttpd_handler_callback_test:112: test_replicate...[0.075 s] ok
chttpd_handler_callback_test:115: test_uuids...[0.076 s] ok
chttpd_handler_callback_test:118: test_sleep...[0.099 s] ok
chttpd_handler_callback_test:121: test_session...[0.007 s] ok
chttpd_handler_callback_test:124: test_oauth...[0.054 s] ok
chttpd_handler_callback_test:127: test_up...[0.076 s] ok
chttpd_handler_callback_test:130: test_membership...[0.018 s] ok
chttpd_handler_callback_test:133: test_db_updates...[0.045 s] ok
chttpd_handler_callback_test:137: test_anything...[0.369 s] ok
chttpd_handler_callback_test:140: verify_unmocked_failing_empty_string...[0.066 s] ok
chttpd_handler_callback_test:143: verify_unmocked_failing_favicon...[0.065 s] ok
chttpd_handler_callback_test:146: verify_unmocked_failing_anything...[0.362 s] ok
chttpd_handler_callback_test:149: test_view_cleanup...[0.367 s] ok
chttpd_handler_callback_test:153: test_compact...[0.372 s] ok
chttpd_handler_callback_test:157: test_design...[0.370 s] ok
chttpd_handler_callback_test:161: test_temp_view...[0.031 s] ok
chttpd_handler_callback_test:165: test_changes...[0.406 s] ok
chttpd_handler_callback_test:169: test_shards...[0.020 s] ok
chttpd_handler_callback_test:173: verify_unmocked_failing_view_cleanup...[0.390 s] ok
chttpd_handler_callback_test:176: verify_unmocked_db_failing_something...ok
chttpd_handler_callback_test:179: test_view...[0.033 s] ok
chttpd_handler_callback_test:183: test_show...[0.051 s] ok
chttpd_handler_callback_test:187: test_list...[0.052 s] ok
chttpd_handler_callback_test:191: test_update...[0.054 s] ok
chttpd_handler_callback_test:195: test_info...[0.393 s] ok
chttpd_handler_callback_test:199: test_rewrite...[0.060 s] ok
chttpd_handler_callback_test:203: verify_unmocked_failing_view...[0.026 s] ok
chttpd_handler_callback_test:206: verify_unmocked_design_failing_something...ok
```
Looks better right? I'd forgot a bit how to make eunit produce the right output: it doesn't works right if setup receives list of functions, so I eventually have to rollback ecd4f77 - sorry for the false tip. Now you see which function got executed and you'll easily know which one fails.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by hdiedrich <gi...@git.apache.org>.
Github user hdiedrich commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#issuecomment-65087557
Ok, thanks! I can set NODEBUG at the top of the test module?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#issuecomment-123569492
Dynamic handlers were landed based on @iilyak work. Could you close this PR please?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#discussion_r21059264
--- Diff: test/chttpd_handler_callback_test.erl ---
@@ -0,0 +1,266 @@
+%% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+%% use this file except in compliance with the License. You may obtain a copy of
+%% the License at
+%%
+%% http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+%% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+%% License for the specific language governing permissions and limitations under
+%% the License.
+%%
+%% @doc
+%% These whitebox tests call the endpoints that are implemented by dynamic funs,
+%% which replace the previously hardwired handlers, with a dynamically created
+%% module. This test mocks everything except the proper relationship between
+%% handler function clause and returned fun. The handler functions are called
+%% directly.
+%%
+%% This can look like mocks testing themselves. In this case, in other words:
+%% think of the former url_handler function in chttpd.erl: The relationship
+%% between a function clause and the specific returned fun() is exactly what is
+%% established by the dynamically created module. And it's precisely what this
+%% test verifies, for every endpoint.
+%%
+%% This test was part of chttpd-cloudant dbcore-2.7.7, verifying the original
+%% hardcoded endpoints. It has since been reworked but remains functionally
+%% mostly identical; the config source and processing has been replaced.
+
+-module(chttpd_handler_callback_test).
+
+-include_lib("eunit/include/eunit.hrl").
+
+all_test_() ->
+ io:format(user, "~nEndpoint handler callbacks:~n", []),
+ {setup,
+ fun() -> chttpd_handler:build(test_cfg()) end,
+ [
+
+ % url handlers
+ fun() ->
+ assertReturns("", chttpd_misc, handle_welcome_req)
+ end,
+ fun() ->
+ assertReturns("favicon.ico", chttpd_misc, handle_favicon_req)
+ end,
+ fun() ->
+ assertReturns("_utils", chttpd_misc, handle_utils_dir_req)
+ end,
+ fun() ->
+ assertReturns("_all_dbs", chttpd_misc, handle_all_dbs_req)
+ end,
+ fun() ->
+ assertReturns("_active_tasks", chttpd_misc, handle_task_status_req)
+ end,
+ fun() ->
+ assertReturns("_config", chttpd_misc, handle_config_req)
+ end,
+ fun() ->
+ assertReturns("_reload_query_servers", chttpd_misc,
+ handle_reload_query_servers_req)
+ end,
+ fun() ->
+ assertReturns("_replicate", chttpd_misc, handle_replicate_req)
+ end,
+ fun() ->
+ assertReturns("_uuids", chttpd_misc, handle_uuids_req)
+ end,
+ fun() ->
+ assertReturns("_sleep", chttpd_misc, handle_sleep_req)
+ end,
+ fun() ->
+ assertReturns("_session", chttpd_auth, handle_session_req)
+ end,
+ fun() ->
+ assertReturns("_oauth", couch_httpd_oauth, handle_oauth_req)
+ end,
+ fun() ->
+ assertReturns("_up", chttpd_misc, handle_up_req)
+ end,
+ fun() ->
+ assertReturns("_membership", mem3_httpd, handle_membership_req)
+ end,
+ fun() ->
+ assertReturns("_db_updates", global_changes_httpd,
+ handle_global_changes_req) end,
+ fun() ->
+ assertReturns("anything", chttpd_db, handle_request)
+ end,
+
+ % Test the tests: if the final target function is missing, the call must
+ % fail, with any parameter. All parameters are valid.
+ fun() ->
+ assertUnmockedFails("", chttpd_misc)
+ end,
+ fun() ->
+ assertUnmockedFails("favicon.ico", chttpd_misc)
+ end,
+ fun() ->
+ assertUnmockedFails(anything, chttpd_db)
+ end,
+
+
+ % db url handler tests
+
+ fun() ->
+ assertReturns(db_url_handlers, <<"_view_cleanup">>, chttpd_db,
+ handle_view_cleanup_req, 2) end,
+ fun() ->
+ assertReturns(db_url_handlers, <<"_compact">>, chttpd_db,
+ handle_compact_req, 2)
+ end,
+ fun() ->
+ assertReturns(db_url_handlers, <<"_design">>, chttpd_db,
+ handle_design_req, 2)
+ end,
+ fun() ->
+ assertReturns(db_url_handlers, <<"_temp_view">>, chttpd_view,
+ handle_temp_view_req, 2) end,
+ fun() ->
+ assertReturns(db_url_handlers, <<"_changes">>, chttpd_db,
+ handle_changes_req, 2) end,
+ fun() ->
+ assertReturns(db_url_handlers, <<"_shards">>, mem3_httpd,
+ handle_shards_req, 2) end,
+
+ % Test the test: when the final target function is missing, the Fun call
+ % must fail, with any argument, including valid ones.
+ fun() ->
+ assertUnmockedFails(db_url_handlers, <<"_view_cleanup">>, chttpd_db, 2)
+ end,
+
+ % Test the test: when the argument is unknown, the Fun call must fail.
+ fun() ->
+ assertUnknownFails(db_url_handlers, <<"_something">>)
+ end,
+
+
+ % design url handler tests
+
+ fun() ->
+ assertReturns(design_url_handlers, <<"_view">>, chttpd_view,
+ handle_view_req, 3) end,
+ fun() ->
+ assertReturns(design_url_handlers, <<"_show">>, chttpd_show,
+ handle_doc_show_req, 3) end,
+ fun() ->
+ assertReturns(design_url_handlers, <<"_list">>, chttpd_show,
+ handle_view_list_req, 3) end,
+ fun() ->
+ assertReturns(design_url_handlers, <<"_update">>, chttpd_show,
+ handle_doc_update_req, 3) end,
+ fun() ->
+ assertReturns(design_url_handlers, <<"_info">>, chttpd_db,
+ handle_design_info_req, 3) end,
+ fun() ->
+ assertReturns(design_url_handlers, <<"_rewrite">>, chttpd_rewrite,
+ handle_rewrite_req, 3) end,
+
+ % Test the test: when the final target function is missing, the Fun call
+ % must fail with any argument, including valid ones.
+ fun() ->
+ assertUnmockedFails(design_url_handlers, <<"_view">>, chttpd_view, 3)
+ end,
+ % Test the test: when the argument is unknown, the Fun call must fail.
+ fun() ->
+ assertUnknownFails(design_url_handlers, <<"_something">>)
+ end]}.
+
+
+%% Call the dynamic function with a parameter known to trigger a specific
+%% clause. Then call the returned fun and get confirmation that this called
+%% the right implicit fun, which is mocked, and returns the expected tuple.
+assertReturns(Endpoint, M, F) ->
+ meck:new(M, [passthrough, non_strict]),
+ try
+ io:format(user, "~-47...s ", [Endpoint]),
+ meck:expect(M, F, fun(X) -> {return, Endpoint, X} end),
+ Fun = chttpd_handler:url_handler(Endpoint),
+ ?assertEqual({return, Endpoint, x}, Fun(x))
--- End diff --
Please don't call ?assert from eunit tests generator: this makes debugging extremely hard:
```
chttpd_handler_callback_test: all_test_.................................................. *failed*
::error:function_clause
in function chttpd_db:handle_request/1
called as handle_request(x)
in call from chttpd_handler_callback_test:'-assertReturns/3-fun-1-'/2
in call from chttpd_handler_callback_test:assertReturns/3
chttpd_handler_callback_test: all_test_...favicon.ico.................................... [0.070 s] ok
chttpd_handler_callback_test: all_test_..._utils......................................... [0.071 s] ok
```
since ?assert* macros checks expression in runtime while ?_assert* doing this when eunit really starts to run the tests + it provides better traceback. See how couchdb-couch tests are done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#discussion_r21059267
--- Diff: test/chttpd_handler_callback_test.erl ---
@@ -0,0 +1,266 @@
+%% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+%% use this file except in compliance with the License. You may obtain a copy of
+%% the License at
+%%
+%% http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+%% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+%% License for the specific language governing permissions and limitations under
+%% the License.
+%%
+%% @doc
+%% These whitebox tests call the endpoints that are implemented by dynamic funs,
+%% which replace the previously hardwired handlers, with a dynamically created
+%% module. This test mocks everything except the proper relationship between
+%% handler function clause and returned fun. The handler functions are called
+%% directly.
+%%
+%% This can look like mocks testing themselves. In this case, in other words:
+%% think of the former url_handler function in chttpd.erl: The relationship
+%% between a function clause and the specific returned fun() is exactly what is
+%% established by the dynamically created module. And it's precisely what this
+%% test verifies, for every endpoint.
+%%
+%% This test was part of chttpd-cloudant dbcore-2.7.7, verifying the original
+%% hardcoded endpoints. It has since been reworked but remains functionally
+%% mostly identical; the config source and processing has been replaced.
+
+-module(chttpd_handler_callback_test).
+
+-include_lib("eunit/include/eunit.hrl").
+
+all_test_() ->
+ io:format(user, "~nEndpoint handler callbacks:~n", []),
--- End diff --
If you need print something to stdout for debug proposes use ?debug macro.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by hdiedrich <gi...@git.apache.org>.
Github user hdiedrich commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#issuecomment-65246798
It's peripheral, I believe, but the output of the test could be helpful to indicate, which endpoint breaks, rather than which test function breaks. That's why I think it may have been more helpful before.
It's not too important maybe, I changed it according to your preference. I certainly see your point.
Would it be welcome to propose eunit formatters?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#discussion_r21059279
--- Diff: test/chttpd_handler_reload_test.erl ---
@@ -0,0 +1,111 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(chttpd_handler_reload_test).
+
+-compile(export_all).
+
+-include_lib("eunit/include/eunit.hrl").
+
+%% for testing
+
+a() -> a.
+b() -> b.
+c() -> c.
+d() -> d.
+e() -> e.
+f() -> f.
+g() -> g.
+h() -> h.
+
+-spec reload_test() -> ok.
+%% @doc verifies that there are no side effects from prior builds.
+reload_test() ->
+
+ % undo any previously loaded test builds
+
+ code:purge(chttpd_dyn_handler), % (must for delete)
+ code:delete(chttpd_dyn_handler),
+ ?assertError(undef, _F0 = chttpd_dyn_handler:clause_test("A")),
+
+ % first build
+
+ Cfg =
+ [{clause_test, clauses, [
+ {"A", {chttpd_handler_reload_test, a, 0}},
+ {"B", {chttpd_handler_reload_test, b, 0}},
+ {'_', {chttpd_handler_reload_test, c, 0}}
+ ]},
+ {list_test, list, [
+ {<<"1">>, {chttpd_handler_reload_test, a, 0}},
+ {<<"2">>, {chttpd_handler_reload_test, b, 0}}
+ ]}],
+
+ chttpd_handler:build(Cfg),
+
+ F1 = chttpd_dyn_handler:clause_test("A"),
+ a = F1(),
+
+ F2 = chttpd_dyn_handler:clause_test("B"),
+ b = F2(),
+
+ F3 = chttpd_dyn_handler:clause_test(xxx),
+ c = F3(),
+
+ {_, F4} = lists:keyfind(<<"1">>, 1, chttpd_dyn_handler:list_test()),
+ a = F4(),
+
+ {_, F5} = lists:keyfind(<<"2">>, 1, chttpd_dyn_handler:list_test()),
+ b = F5(),
+
+ % second build, expected to overwrite the earlier
+
+ Cfg2 =
+ [{clause_test, clauses, [
+ {"D", {chttpd_handler_reload_test, d, 0}},
+ {"E", {chttpd_handler_reload_test, e, 0}},
+ {'_', {chttpd_handler_reload_test, f, 0}}
+ ]},
+ {list_test, list, [
+ {<<"3">>, {chttpd_handler_reload_test, g, 0}},
+ {<<"4">>, {chttpd_handler_reload_test, h, 0}}
+ ]}],
+
+ chttpd_handler:build(Cfg2),
+
+ F6 = chttpd_dyn_handler:clause_test("A"),
+ io:format("~p~n", [F6]),
+ ?assertError({badmatch, f}, a = F6()),
+
+ F7 = chttpd_dyn_handler:clause_test("B"),
+ ?assertError({badmatch, f}, b = F7()),
+
+ F8 = chttpd_dyn_handler:clause_test("D"),
+ d = F8(),
+
+ F9 = chttpd_dyn_handler:clause_test("E"),
+ e = F9(),
+
+ F10 = chttpd_dyn_handler:clause_test(yyy),
+ f = F10(),
+
+ false = lists:keyfind(<<"1">>, 1, chttpd_dyn_handler:list_test()),
--- End diff --
If this is a part of test, use ?assert here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by hdiedrich <gi...@git.apache.org>.
Github user hdiedrich commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#issuecomment-64922076
Note, I am adding less essential stuff, if unplanned for, in separate commits, as requested.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#issuecomment-101317048
@hdiedrich Hey! Config support suddenly is needed by the following reasons:
- It saved us many times when security issue was found in some HTTP handler. The recent example: http://docs.couchdb.org/en/latest/cve/2014-2668.html - users were able to handle that problem without upgrade procedure, but with a single config line change.
- Some users are disables default handlers, or provide own overlay, in order to prevent or customize access to certain endpoints. Quite popular move is to disable /_all_dbs in order to prevent sharing database lists for everyone. Another option is to replace default handler with own from the plugin if you're going to change behaviour without patching the source code.
- Somehow need to provide ability to register proxy and external handlers.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#discussion_r21059266
--- Diff: test/chttpd_handler_callback_test.erl ---
@@ -0,0 +1,266 @@
+%% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+%% use this file except in compliance with the License. You may obtain a copy of
+%% the License at
+%%
+%% http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+%% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+%% License for the specific language governing permissions and limitations under
+%% the License.
+%%
+%% @doc
+%% These whitebox tests call the endpoints that are implemented by dynamic funs,
+%% which replace the previously hardwired handlers, with a dynamically created
+%% module. This test mocks everything except the proper relationship between
+%% handler function clause and returned fun. The handler functions are called
+%% directly.
+%%
+%% This can look like mocks testing themselves. In this case, in other words:
+%% think of the former url_handler function in chttpd.erl: The relationship
+%% between a function clause and the specific returned fun() is exactly what is
+%% established by the dynamically created module. And it's precisely what this
+%% test verifies, for every endpoint.
+%%
+%% This test was part of chttpd-cloudant dbcore-2.7.7, verifying the original
+%% hardcoded endpoints. It has since been reworked but remains functionally
+%% mostly identical; the config source and processing has been replaced.
+
+-module(chttpd_handler_callback_test).
+
+-include_lib("eunit/include/eunit.hrl").
+
+all_test_() ->
+ io:format(user, "~nEndpoint handler callbacks:~n", []),
+ {setup,
+ fun() -> chttpd_handler:build(test_cfg()) end,
+ [
--- End diff --
If you replace anonymous functions with real ones you'll have no need to reinvent you own debugging printing since eunit will handle this well and right.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#issuecomment-65086583
> however, do we need to have "httpd_handler_callback_test:206:" before each output line?
That will save a lot of time if something will go wrong since it directly points on the exact line in exact module where to start your search.
> I don't know what apache/couchdb-chttpd does differently with the eunit setup but the same test source did not print these module:line hints before every line.
IIRC that's some eunit internal thing about using `setup`. I'd fight with the output year ago and might be forgot all the details, but one I know for sure: it's an easy to produce useless test report with eunit. Need to note, that I'm care here about further test suite support.
> Also, I am using io:format to prevent even more prefix in each line and I will try out your setup but all I ever found was that ?debugFmt inevitably adds to the line. I did not invent using io:format(user, ... that is from the Eunit docs, the way to produce screen output.
Sorry if "reinvent" was sound a bit rude. The problem of io:format that it will always print things to stdout. As for ?debug, you can turn this output off after defining NODEBUG leaving whole report clean.
> I polished this output to be concise because I needed to find a race condition that only became apparent in the logs.
Sounds like you'll have to add another case on that (; Good luck with chasing RC - that's not an easy.
> From that I got to appreciate briefer, readable, cleaned up logs and modelled it after the etap output.
I see. Well, for eunit other tests are already uses the "default" output format, so if you think it could be improved, it need to be improved everywhere.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by hdiedrich <gi...@git.apache.org>.
Github user hdiedrich commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#discussion_r21090978
--- Diff: test/chttpd_handler_callback_test.erl ---
@@ -0,0 +1,266 @@
+%% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+%% use this file except in compliance with the License. You may obtain a copy of
+%% the License at
+%%
+%% http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+%% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+%% License for the specific language governing permissions and limitations under
+%% the License.
+%%
+%% @doc
+%% These whitebox tests call the endpoints that are implemented by dynamic funs,
+%% which replace the previously hardwired handlers, with a dynamically created
+%% module. This test mocks everything except the proper relationship between
+%% handler function clause and returned fun. The handler functions are called
+%% directly.
+%%
+%% This can look like mocks testing themselves. In this case, in other words:
+%% think of the former url_handler function in chttpd.erl: The relationship
+%% between a function clause and the specific returned fun() is exactly what is
+%% established by the dynamically created module. And it's precisely what this
+%% test verifies, for every endpoint.
+%%
+%% This test was part of chttpd-cloudant dbcore-2.7.7, verifying the original
+%% hardcoded endpoints. It has since been reworked but remains functionally
+%% mostly identical; the config source and processing has been replaced.
+
+-module(chttpd_handler_callback_test).
+
+-include_lib("eunit/include/eunit.hrl").
+
+all_test_() ->
+ io:format(user, "~nEndpoint handler callbacks:~n", []),
+ {setup,
+ fun() -> chttpd_handler:build(test_cfg()) end,
+ [
--- End diff --
The new commit addresses this and changes all anonymous functions to real ones.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#issuecomment-65291589
> It's peripheral, I believe, but the output of the test could be helpful to indicate, which endpoint breaks, rather than which test function breaks. That's why I think it may have been more helpful before.
If function name describes what exactly it tests (which endpoint for what behavior in your case), you'll hit two birds with single stone (;
> Would it be welcome to propose eunit formatters?
I'm +1 on this. Dream on something colorful with slow tests highlight and nicer function name transforming into more readable form - that would be awesome!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#discussion_r21059280
--- Diff: test/chttpd_handler_reload_test.erl ---
@@ -0,0 +1,111 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(chttpd_handler_reload_test).
+
+-compile(export_all).
+
+-include_lib("eunit/include/eunit.hrl").
+
+%% for testing
+
+a() -> a.
+b() -> b.
+c() -> c.
+d() -> d.
+e() -> e.
+f() -> f.
+g() -> g.
+h() -> h.
+
+-spec reload_test() -> ok.
+%% @doc verifies that there are no side effects from prior builds.
+reload_test() ->
+
+ % undo any previously loaded test builds
+
+ code:purge(chttpd_dyn_handler), % (must for delete)
+ code:delete(chttpd_dyn_handler),
+ ?assertError(undef, _F0 = chttpd_dyn_handler:clause_test("A")),
+
+ % first build
+
+ Cfg =
+ [{clause_test, clauses, [
+ {"A", {chttpd_handler_reload_test, a, 0}},
+ {"B", {chttpd_handler_reload_test, b, 0}},
+ {'_', {chttpd_handler_reload_test, c, 0}}
+ ]},
+ {list_test, list, [
+ {<<"1">>, {chttpd_handler_reload_test, a, 0}},
+ {<<"2">>, {chttpd_handler_reload_test, b, 0}}
+ ]}],
+
+ chttpd_handler:build(Cfg),
+
+ F1 = chttpd_dyn_handler:clause_test("A"),
+ a = F1(),
+
+ F2 = chttpd_dyn_handler:clause_test("B"),
+ b = F2(),
+
+ F3 = chttpd_dyn_handler:clause_test(xxx),
+ c = F3(),
+
+ {_, F4} = lists:keyfind(<<"1">>, 1, chttpd_dyn_handler:list_test()),
+ a = F4(),
+
+ {_, F5} = lists:keyfind(<<"2">>, 1, chttpd_dyn_handler:list_test()),
+ b = F5(),
+
+ % second build, expected to overwrite the earlier
+
+ Cfg2 =
+ [{clause_test, clauses, [
+ {"D", {chttpd_handler_reload_test, d, 0}},
+ {"E", {chttpd_handler_reload_test, e, 0}},
+ {'_', {chttpd_handler_reload_test, f, 0}}
+ ]},
+ {list_test, list, [
+ {<<"3">>, {chttpd_handler_reload_test, g, 0}},
+ {<<"4">>, {chttpd_handler_reload_test, h, 0}}
+ ]}],
+
+ chttpd_handler:build(Cfg2),
+
+ F6 = chttpd_dyn_handler:clause_test("A"),
+ io:format("~p~n", [F6]),
--- End diff --
?debug for printing.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#issuecomment-65088688
Yes, but better add `{erl_opts, [{d, NODEBUG}]}.` to rebar config (setup_eunit?). This better have global wide.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#issuecomment-97832439
@hdiedrich: Any updates on this PR?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by hdiedrich <gi...@git.apache.org>.
Github user hdiedrich commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#discussion_r21085923
--- Diff: test/chttpd_handler_callback_test.erl ---
@@ -0,0 +1,266 @@
+%% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+%% use this file except in compliance with the License. You may obtain a copy of
+%% the License at
+%%
+%% http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+%% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+%% License for the specific language governing permissions and limitations under
+%% the License.
+%%
+%% @doc
+%% These whitebox tests call the endpoints that are implemented by dynamic funs,
+%% which replace the previously hardwired handlers, with a dynamically created
+%% module. This test mocks everything except the proper relationship between
+%% handler function clause and returned fun. The handler functions are called
+%% directly.
+%%
+%% This can look like mocks testing themselves. In this case, in other words:
+%% think of the former url_handler function in chttpd.erl: The relationship
+%% between a function clause and the specific returned fun() is exactly what is
+%% established by the dynamically created module. And it's precisely what this
+%% test verifies, for every endpoint.
+%%
+%% This test was part of chttpd-cloudant dbcore-2.7.7, verifying the original
+%% hardcoded endpoints. It has since been reworked but remains functionally
+%% mostly identical; the config source and processing has been replaced.
+
+-module(chttpd_handler_callback_test).
+
+-include_lib("eunit/include/eunit.hrl").
+
+all_test_() ->
+ io:format(user, "~nEndpoint handler callbacks:~n", []),
+ {setup,
+ fun() -> chttpd_handler:build(test_cfg()) end,
+ [
--- End diff --
Thanks for the review!
Do you mean: replacing the individual test functions, e.g. changing
```erlang
fun() ->
assertReturns("", chttpd_misc, handle_welcome_req)
end,
```
to something like `test_chttpd_misc_handle_welcome_req/1` with a separate definition in the module body
```erlang
test_chttpd_misc_handle_welcome_req() ->
assertReturns("", chttpd_misc, handle_welcome_req)
end.
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#discussion_r21087563
--- Diff: test/chttpd_handler_reload_test.erl ---
@@ -0,0 +1,111 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(chttpd_handler_reload_test).
+
+-compile(export_all).
+
+-include_lib("eunit/include/eunit.hrl").
+
+%% for testing
+
+a() -> a.
+b() -> b.
+c() -> c.
+d() -> d.
+e() -> e.
+f() -> f.
+g() -> g.
+h() -> h.
+
+-spec reload_test() -> ok.
+%% @doc verifies that there are no side effects from prior builds.
+reload_test() ->
--- End diff --
> I'll be happy to discuss, maybe on IRC?
Sure, I'll be on #couchdb-dev / #couchdb closer to 17:00 UTC.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by hdiedrich <gi...@git.apache.org>.
Github user hdiedrich commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#issuecomment-65216944
I guess the NODEBUG proposal was a misunderstanding. I was looking for a way to create readable test output, as created during the test. Not sure what you actually meant with using NODEBUG, will leave that out for now.
I'll otherwise rewrite as requested, to move forward. For my part I am unconvinced the test output needs line numbers added as it clutters the test source a bit, not making it more readable and adding a runtime indirection, if I understand that correctly, away from the intuitive assumption, for what it's worth. And adding the additional setup time using foreach reduces the test speed, for little gain, for all I can see.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by hdiedrich <gi...@git.apache.org>.
Github user hdiedrich commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#discussion_r21085971
--- Diff: test/chttpd_handler_reload_test.erl ---
@@ -0,0 +1,111 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(chttpd_handler_reload_test).
+
+-compile(export_all).
+
+-include_lib("eunit/include/eunit.hrl").
+
+%% for testing
+
+a() -> a.
+b() -> b.
+c() -> c.
+d() -> d.
+e() -> e.
+f() -> f.
+g() -> g.
+h() -> h.
+
+-spec reload_test() -> ok.
+%% @doc verifies that there are no side effects from prior builds.
+reload_test() ->
+
+ % undo any previously loaded test builds
+
+ code:purge(chttpd_dyn_handler), % (must for delete)
+ code:delete(chttpd_dyn_handler),
+ ?assertError(undef, _F0 = chttpd_dyn_handler:clause_test("A")),
+
+ % first build
+
+ Cfg =
+ [{clause_test, clauses, [
+ {"A", {chttpd_handler_reload_test, a, 0}},
+ {"B", {chttpd_handler_reload_test, b, 0}},
+ {'_', {chttpd_handler_reload_test, c, 0}}
+ ]},
+ {list_test, list, [
+ {<<"1">>, {chttpd_handler_reload_test, a, 0}},
+ {<<"2">>, {chttpd_handler_reload_test, b, 0}}
+ ]}],
+
+ chttpd_handler:build(Cfg),
+
+ F1 = chttpd_dyn_handler:clause_test("A"),
+ a = F1(),
+
+ F2 = chttpd_dyn_handler:clause_test("B"),
+ b = F2(),
+
+ F3 = chttpd_dyn_handler:clause_test(xxx),
+ c = F3(),
+
+ {_, F4} = lists:keyfind(<<"1">>, 1, chttpd_dyn_handler:list_test()),
+ a = F4(),
+
+ {_, F5} = lists:keyfind(<<"2">>, 1, chttpd_dyn_handler:list_test()),
+ b = F5(),
+
+ % second build, expected to overwrite the earlier
+
+ Cfg2 =
+ [{clause_test, clauses, [
+ {"D", {chttpd_handler_reload_test, d, 0}},
+ {"E", {chttpd_handler_reload_test, e, 0}},
+ {'_', {chttpd_handler_reload_test, f, 0}}
+ ]},
+ {list_test, list, [
+ {<<"3">>, {chttpd_handler_reload_test, g, 0}},
+ {<<"4">>, {chttpd_handler_reload_test, h, 0}}
+ ]}],
+
+ chttpd_handler:build(Cfg2),
+
+ F6 = chttpd_dyn_handler:clause_test("A"),
+ io:format("~p~n", [F6]),
+ ?assertError({badmatch, f}, a = F6()),
+
+ F7 = chttpd_dyn_handler:clause_test("B"),
+ ?assertError({badmatch, f}, b = F7()),
+
+ F8 = chttpd_dyn_handler:clause_test("D"),
+ d = F8(),
+
+ F9 = chttpd_dyn_handler:clause_test("E"),
+ e = F9(),
+
+ F10 = chttpd_dyn_handler:clause_test(yyy),
+ f = F10(),
+
+ false = lists:keyfind(<<"1">>, 1, chttpd_dyn_handler:list_test()),
--- End diff --
This is now changed, thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by hdiedrich <gi...@git.apache.org>.
Github user hdiedrich commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#discussion_r21085948
--- Diff: test/chttpd_handler_reload_test.erl ---
@@ -0,0 +1,111 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(chttpd_handler_reload_test).
+
+-compile(export_all).
+
+-include_lib("eunit/include/eunit.hrl").
+
+%% for testing
+
+a() -> a.
+b() -> b.
+c() -> c.
+d() -> d.
+e() -> e.
+f() -> f.
+g() -> g.
+h() -> h.
+
+-spec reload_test() -> ok.
+%% @doc verifies that there are no side effects from prior builds.
+reload_test() ->
+
+ % undo any previously loaded test builds
+
+ code:purge(chttpd_dyn_handler), % (must for delete)
+ code:delete(chttpd_dyn_handler),
+ ?assertError(undef, _F0 = chttpd_dyn_handler:clause_test("A")),
+
+ % first build
+
+ Cfg =
+ [{clause_test, clauses, [
+ {"A", {chttpd_handler_reload_test, a, 0}},
+ {"B", {chttpd_handler_reload_test, b, 0}},
+ {'_', {chttpd_handler_reload_test, c, 0}}
+ ]},
+ {list_test, list, [
+ {<<"1">>, {chttpd_handler_reload_test, a, 0}},
+ {<<"2">>, {chttpd_handler_reload_test, b, 0}}
+ ]}],
+
+ chttpd_handler:build(Cfg),
+
+ F1 = chttpd_dyn_handler:clause_test("A"),
+ a = F1(),
+
+ F2 = chttpd_dyn_handler:clause_test("B"),
+ b = F2(),
+
+ F3 = chttpd_dyn_handler:clause_test(xxx),
+ c = F3(),
+
+ {_, F4} = lists:keyfind(<<"1">>, 1, chttpd_dyn_handler:list_test()),
+ a = F4(),
+
+ {_, F5} = lists:keyfind(<<"2">>, 1, chttpd_dyn_handler:list_test()),
+ b = F5(),
+
+ % second build, expected to overwrite the earlier
+
+ Cfg2 =
+ [{clause_test, clauses, [
+ {"D", {chttpd_handler_reload_test, d, 0}},
+ {"E", {chttpd_handler_reload_test, e, 0}},
+ {'_', {chttpd_handler_reload_test, f, 0}}
+ ]},
+ {list_test, list, [
+ {<<"3">>, {chttpd_handler_reload_test, g, 0}},
+ {<<"4">>, {chttpd_handler_reload_test, h, 0}}
+ ]}],
+
+ chttpd_handler:build(Cfg2),
+
+ F6 = chttpd_dyn_handler:clause_test("A"),
+ io:format("~p~n", [F6]),
--- End diff --
This line should not have remained.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by hdiedrich <gi...@git.apache.org>.
Github user hdiedrich commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#discussion_r21086105
--- Diff: test/chttpd_handler_callback_test.erl ---
@@ -0,0 +1,266 @@
+%% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+%% use this file except in compliance with the License. You may obtain a copy of
+%% the License at
+%%
+%% http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+%% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+%% License for the specific language governing permissions and limitations under
+%% the License.
+%%
+%% @doc
+%% These whitebox tests call the endpoints that are implemented by dynamic funs,
+%% which replace the previously hardwired handlers, with a dynamically created
+%% module. This test mocks everything except the proper relationship between
+%% handler function clause and returned fun. The handler functions are called
+%% directly.
+%%
+%% This can look like mocks testing themselves. In this case, in other words:
+%% think of the former url_handler function in chttpd.erl: The relationship
+%% between a function clause and the specific returned fun() is exactly what is
+%% established by the dynamically created module. And it's precisely what this
+%% test verifies, for every endpoint.
+%%
+%% This test was part of chttpd-cloudant dbcore-2.7.7, verifying the original
+%% hardcoded endpoints. It has since been reworked but remains functionally
+%% mostly identical; the config source and processing has been replaced.
+
+-module(chttpd_handler_callback_test).
+
+-include_lib("eunit/include/eunit.hrl").
+
+all_test_() ->
+ io:format(user, "~nEndpoint handler callbacks:~n", []),
+ {setup,
+ fun() -> chttpd_handler:build(test_cfg()) end,
+ [
+
+ % url handlers
+ fun() ->
+ assertReturns("", chttpd_misc, handle_welcome_req)
+ end,
+ fun() ->
+ assertReturns("favicon.ico", chttpd_misc, handle_favicon_req)
+ end,
+ fun() ->
+ assertReturns("_utils", chttpd_misc, handle_utils_dir_req)
+ end,
+ fun() ->
+ assertReturns("_all_dbs", chttpd_misc, handle_all_dbs_req)
+ end,
+ fun() ->
+ assertReturns("_active_tasks", chttpd_misc, handle_task_status_req)
+ end,
+ fun() ->
+ assertReturns("_config", chttpd_misc, handle_config_req)
+ end,
+ fun() ->
+ assertReturns("_reload_query_servers", chttpd_misc,
+ handle_reload_query_servers_req)
+ end,
+ fun() ->
+ assertReturns("_replicate", chttpd_misc, handle_replicate_req)
+ end,
+ fun() ->
+ assertReturns("_uuids", chttpd_misc, handle_uuids_req)
+ end,
+ fun() ->
+ assertReturns("_sleep", chttpd_misc, handle_sleep_req)
+ end,
+ fun() ->
+ assertReturns("_session", chttpd_auth, handle_session_req)
+ end,
+ fun() ->
+ assertReturns("_oauth", couch_httpd_oauth, handle_oauth_req)
+ end,
+ fun() ->
+ assertReturns("_up", chttpd_misc, handle_up_req)
+ end,
+ fun() ->
+ assertReturns("_membership", mem3_httpd, handle_membership_req)
+ end,
+ fun() ->
+ assertReturns("_db_updates", global_changes_httpd,
+ handle_global_changes_req) end,
+ fun() ->
+ assertReturns("anything", chttpd_db, handle_request)
+ end,
+
+ % Test the tests: if the final target function is missing, the call must
+ % fail, with any parameter. All parameters are valid.
+ fun() ->
+ assertUnmockedFails("", chttpd_misc)
+ end,
+ fun() ->
+ assertUnmockedFails("favicon.ico", chttpd_misc)
+ end,
+ fun() ->
+ assertUnmockedFails(anything, chttpd_db)
+ end,
+
+
+ % db url handler tests
+
+ fun() ->
+ assertReturns(db_url_handlers, <<"_view_cleanup">>, chttpd_db,
+ handle_view_cleanup_req, 2) end,
+ fun() ->
+ assertReturns(db_url_handlers, <<"_compact">>, chttpd_db,
+ handle_compact_req, 2)
+ end,
+ fun() ->
+ assertReturns(db_url_handlers, <<"_design">>, chttpd_db,
+ handle_design_req, 2)
+ end,
+ fun() ->
+ assertReturns(db_url_handlers, <<"_temp_view">>, chttpd_view,
+ handle_temp_view_req, 2) end,
+ fun() ->
+ assertReturns(db_url_handlers, <<"_changes">>, chttpd_db,
+ handle_changes_req, 2) end,
+ fun() ->
+ assertReturns(db_url_handlers, <<"_shards">>, mem3_httpd,
+ handle_shards_req, 2) end,
+
+ % Test the test: when the final target function is missing, the Fun call
+ % must fail, with any argument, including valid ones.
+ fun() ->
+ assertUnmockedFails(db_url_handlers, <<"_view_cleanup">>, chttpd_db, 2)
+ end,
+
+ % Test the test: when the argument is unknown, the Fun call must fail.
+ fun() ->
+ assertUnknownFails(db_url_handlers, <<"_something">>)
+ end,
+
+
+ % design url handler tests
+
+ fun() ->
+ assertReturns(design_url_handlers, <<"_view">>, chttpd_view,
+ handle_view_req, 3) end,
+ fun() ->
+ assertReturns(design_url_handlers, <<"_show">>, chttpd_show,
+ handle_doc_show_req, 3) end,
+ fun() ->
+ assertReturns(design_url_handlers, <<"_list">>, chttpd_show,
+ handle_view_list_req, 3) end,
+ fun() ->
+ assertReturns(design_url_handlers, <<"_update">>, chttpd_show,
+ handle_doc_update_req, 3) end,
+ fun() ->
+ assertReturns(design_url_handlers, <<"_info">>, chttpd_db,
+ handle_design_info_req, 3) end,
+ fun() ->
+ assertReturns(design_url_handlers, <<"_rewrite">>, chttpd_rewrite,
+ handle_rewrite_req, 3) end,
+
+ % Test the test: when the final target function is missing, the Fun call
+ % must fail with any argument, including valid ones.
+ fun() ->
+ assertUnmockedFails(design_url_handlers, <<"_view">>, chttpd_view, 3)
+ end,
+ % Test the test: when the argument is unknown, the Fun call must fail.
+ fun() ->
+ assertUnknownFails(design_url_handlers, <<"_something">>)
+ end]}.
+
+
+%% Call the dynamic function with a parameter known to trigger a specific
+%% clause. Then call the returned fun and get confirmation that this called
+%% the right implicit fun, which is mocked, and returns the expected tuple.
+assertReturns(Endpoint, M, F) ->
+ meck:new(M, [passthrough, non_strict]),
+ try
+ io:format(user, "~-47...s ", [Endpoint]),
+ meck:expect(M, F, fun(X) -> {return, Endpoint, X} end),
+ Fun = chttpd_handler:url_handler(Endpoint),
+ ?assertEqual({return, Endpoint, x}, Fun(x))
--- End diff --
This is now changed, thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#discussion_r21087513
--- Diff: test/chttpd_handler_reload_test.erl ---
@@ -0,0 +1,111 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(chttpd_handler_reload_test).
+
+-compile(export_all).
+
+-include_lib("eunit/include/eunit.hrl").
+
+%% for testing
+
+a() -> a.
+b() -> b.
+c() -> c.
+d() -> d.
+e() -> e.
+f() -> f.
+g() -> g.
+h() -> h.
+
+-spec reload_test() -> ok.
+%% @doc verifies that there are no side effects from prior builds.
+reload_test() ->
--- End diff --
As I can see the code, you can put firing up cluster into setup/teardown functions and using for/3 apply it over the cases you have there:
- check endpoint by exact name ("A" and "B")
- check endpoint by any name ('_' case)
- find 1 and 2 in list (not sure what the case is described there)
- check that endpoints could be overridden correctly
- check that list things could be overridden correctly
So at least there are 5 different cases in the single function. And if someone will broke up it would be easy to track the problem down and fix it without touching other cases.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#discussion_r23889246
--- Diff: src/chttpd_handler.erl ---
@@ -0,0 +1,202 @@
+%% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+%% use this file except in compliance with the License. You may obtain a copy of
+%% the License at
+%%
+%% http://www.apache.org/licenses/LICENSE-2.0
+%%
+%% Unless required by applicable law or agreed to in writing, software
+%% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+%% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+%% License for the specific language governing permissions and limitations under
+%% the License.
+
+%% @doc Configurable, dynamic creation of endpoint handler callback indirections.
+
+-module(chttpd_handler).
+
+-export([build/0, build/1, url_handler/1, db_url_handlers/0,
+ design_url_handlers/0]).
+
+-vsn(4).
+
+%% @doc a complete configuration data set
+-type config() :: [Function::{Name::atom(), clauses|list, [bind()]}].
+
+%% @doc one essential pair of a pattern and the fun to be returned for it
+-type bind() :: {Endpoint::term(), MFA::{atom(), atom(), integer()}}.
+
+-spec url_handler(Endpoint::list()) -> Handler::fun().
+%% @doc Dispatch endpoint to fun, wrapper to hide dynamic module.
+url_handler(Endpoint) ->
+ chttpd_dyn_handler:url_handler(Endpoint).
+
+-spec db_url_handlers() -> [{Endpoint::list(), Handler::fun()}].
+%% @doc Get a list of endpoints and handler funs, wrapper to hide dyn module.
+db_url_handlers() ->
+ chttpd_dyn_handler:db_url_handlers().
+
+-spec design_url_handlers() -> [{Endpoint::list(), Handler::fun()}].
+%% @doc Get a list of endpoints and handler funs, wrapper to hide dyn module.
+design_url_handlers() ->
+ chttpd_dyn_handler:design_url_handlers().
+
+-spec build() -> ok | [].
+%% @doc Create the dynamic handler functions from ini file.
+build() ->
+ build(load_defs()).
+
+-spec build(HandlerCfg::config()) -> ok.
+%% @doc Compile the complete syntax tree, purge and load the dynamic module
+build(Cfg) when is_list(Cfg) ->
+ Opts = [verbose, report_errors],
+ {ok, Mod, Bin} = compile:forms(forms(chttpd_dyn_handler, Cfg), Opts),
+ % don't code:purge(Mod),
+ {module, Mod} = code:load_binary(Mod, atom_to_list(Mod) ++ ".erl", Bin),
+ ok.
+
+-spec load_defs() -> CombinedHandlerCfg::config().
+%% @doc assemble the configuration from the chttpd_handler.cfg of all apps.
+load_defs() ->
+ {AllURLHandlers, AllDBHandlers, AllDesignHandlers} = lists:foldl(
+ fun(App, {URLHandlers, DBHandlers, DesignHandlers}) ->
+ Defs = load_defs(App),
+ {URLHandlers ++ [ B || {docs, clauses, B} <- Defs ],
+ DBHandlers ++ [ B || {db, list, B} <- Defs ],
+ DesignHandlers ++ [ B || {design, list, B} <- Defs ]}
+ end,
+ {[],[],[]},
+ [element(1, A) || A <- application:loaded_applications()]),
+ [{url_handler, clauses, lists:flatten(AllURLHandlers)},
+ {db_url_handlers, list, lists:flatten(AllDBHandlers)},
+ {design_url_handlers, list, lists:flatten(AllDesignHandlers)}].
+
+-spec load_defs(AppName::atom()) -> OneAppsHandlerCfg::config().
+%% @doc assemble the configuration from the chttpd_handler.cfg of all apps.
+load_defs(App) ->
+ case code:priv_dir(App) of
+ {error, _Error} ->
+ [];
+ Dir ->
+ Path = Dir ++ "/chttpd_handler.cfg",
--- End diff --
Seems I miss that moment. Why not to reuse `config` and have all the handlers defined there? We already keep httpd ones there and this gives us dynamic control over them with no server restart.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by hdiedrich <gi...@git.apache.org>.
Github user hdiedrich commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#issuecomment-65084297
Thanks a lot for digging into it, I'll change accordingly, however, do we need to have "httpd_handler_callback_test:206:" before each output line? I don't know what apache/couchdb-chttpd does differently with the eunit setup but the same test source did not print these module:line hints before every line.
Also, I am using io:format to prevent even more prefix in each line and I will try out your setup but all I ever found was that ?debugFmt inevitably adds to the line. I did not invent using io:format(user, ... that is from the Eunit docs, the way to produce screen output.
I polished this output to be concise because I needed to find a race condition that only became apparent in the logs. From that I got to appreciate briefer, readable, cleaned up logs and modelled it after the etap output. I will try your proposal and go from there, then come back with what I found regarding ?debugFmt and leave it up to you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#discussion_r21059277
--- Diff: test/chttpd_handler_reload_test.erl ---
@@ -0,0 +1,111 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(chttpd_handler_reload_test).
+
+-compile(export_all).
+
+-include_lib("eunit/include/eunit.hrl").
+
+%% for testing
+
+a() -> a.
+b() -> b.
+c() -> c.
+d() -> d.
+e() -> e.
+f() -> f.
+g() -> g.
+h() -> h.
+
+-spec reload_test() -> ok.
+%% @doc verifies that there are no side effects from prior builds.
+reload_test() ->
--- End diff --
This test case looks too global. Please split it for multiple functions which are tests own small specific cases. We're already had one-function-to-test-everything with etap test suite and this doesn't works.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#issuecomment-115216196
The plan that the couchdb 2.0 sprint team agreed on is basically;
1) every application can easily register its own endpoints simply by existing
2) administrators can disable any endpoints they like
3) the configuration file's proxy and external handlers section should be included in the set of endpoints (and added last)
As to how we can support 'overlay' of an application's endpoints, I'm less sure. I suggest we get the above steps done first and then reassess.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#discussion_r21099269
--- Diff: test/chttpd_handler_reload_test.erl ---
@@ -0,0 +1,111 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(chttpd_handler_reload_test).
+
+-compile(export_all).
+
+-include_lib("eunit/include/eunit.hrl").
+
+%% for testing
+
+a() -> a.
+b() -> b.
+c() -> c.
+d() -> d.
+e() -> e.
+f() -> f.
+g() -> g.
+h() -> h.
+
+-spec reload_test() -> ok.
+%% @doc verifies that there are no side effects from prior builds.
+reload_test() ->
--- End diff --
The value in splitting this out is that we will know the full pass/fail matrix for each of the tests. Keeping them together like this means we'll only know the first failure. I think that's a useful change if it doesn't make the test too much more complicated. I don't think it's *required* to split these out given how unlikely breaking one (but not all) is, so I'm +1 on this test as it stands and +1 on improving it if possible.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by hdiedrich <gi...@git.apache.org>.
Github user hdiedrich commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#issuecomment-101308611
Hi, I need to review what that over all situation for this is at this point. Status is that there was a wish from Paul on how to better kick the handlers in, which I liked, but got stuck implementing it and kept going backwards finding out why until it became clear that I need to port my system tests over from the original implementation in dbcore that had met little love there. I then waited for integration tests from Russel and Ilya to be merged, so I don't double their effort but those got stuck, too IIRC. The way forward should be simple and fast and I'll be happy to finally get it merged.
The start up, as it is, has the weakness that there needs to be knowledge about the modules in play, which defeats the intended uncoupling. The solution would be to just re-compile every step of the way when a new app is added during start up. That makes it fool proof and self-sufficient. Config, as Alex suggests, should therefore not be needed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-chttpd pull request: Dynamic endpoints handlers
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/10#issuecomment-65217771
@hdiedrich if you're looking for fixing eunit output take a look on how formatters are done for it. For instance: https://github.com/seancribbs/eunit_formatters . Doing the same work with io:format isn't right in anyway.
> For my part I am unconvinced the test output needs line numbers added as it clutters the test source a bit.
Remove this info, break the tests, give someone to fix them and ask does this info could make his live simpler or not (: Chasing readability shouldn't hurt maintenance support.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---