You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by iilyak <gi...@git.apache.org> on 2015/08/25 02:42:28 UTC
[GitHub] couchdb-chttpd pull request: Refactor test suite for httpd endpoin...
GitHub user iilyak opened a pull request:
https://github.com/apache/couchdb-chttpd/pull/65
Refactor test suite for httpd endpoints
Split out common parts into util module so we can reuse test suite.
The util module could be used for testing:
- endpoints defined in other applications
- overridden endpoints
COUCHDB-2789
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cloudant/couchdb-chttpd 2789-httpd_endpoints_testsuite
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-chttpd/pull/65.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 #65
----
commit 2ecfdab3e89535a519a82f72795b9979c3269a7f
Author: ILYA Khlopotov <ii...@ca.ibm.com>
Date: 2015-08-25T00:16:56Z
Refactor test suite for httpd endpoints
Split out common parts into util module so we can reuse test suite.
The util module could be used for testing:
- endpoints defined in other applications
- overridden endpoints
COUCHDB-2789
----
---
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 issue #65: Refactor test suite for httpd endpoints
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on the issue:
https://github.com/apache/couchdb-chttpd/pull/65
stale PR
---
[GitHub] couchdb-chttpd pull request: Refactor test suite for httpd endpoin...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/65#issuecomment-135046143
I think it's good and helpful for introspection proposes. And one more step closer to the same behavior as `httpd_*_handlers` config options provides now (:
---
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: Refactor test suite for httpd endpoin...
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/65#discussion_r37904391
--- Diff: test/chttpd_httpd_handlers_tests.erl ---
@@ -0,0 +1,54 @@
+% 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_httpd_handlers_tests).
+
+-export([handlers/1]).
+
+-include_lib("couch/include/couch_eunit.hrl").
+
+handlers(url_handler) ->
+ [
+ {<<"">>, chttpd_misc, handle_welcome_req},
+ {<<"favicon.ico">>, chttpd_misc, handle_favicon_req},
+ {<<"_utils">>, chttpd_misc, handle_utils_dir_req},
+ {<<"_all_dbs">>, chttpd_misc, handle_all_dbs_req},
+ {<<"_active_tasks">>, chttpd_misc, handle_task_status_req},
+ {<<"_node">>, chttpd_misc, handle_node_req},
+ {<<"_reload_query_servers">>, chttpd_misc, handle_reload_query_servers_req},
+ {<<"_replicate">>, chttpd_misc, handle_replicate_req},
+ {<<"_uuids">>, chttpd_misc, handle_uuids_req},
+ {<<"_session">>, chttpd_auth, handle_session_req},
+ {<<"_up">>, chttpd_misc, handle_up_req},
+ {<<"anything">>, chttpd_db, handle_request}
--- End diff --
On a side note. I am also thinking rather it would be better to place tests in `<app>_httpd_handlers.erl` itself.
---
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: Refactor test suite for httpd endpoin...
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/65#discussion_r37846721
--- Diff: src/chttpd_httpd_handlers_test_util.erl ---
@@ -0,0 +1,200 @@
+% 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_httpd_handlers_test_util).
+
+-export([endpoints_test/3]).
+
+-include_lib("couch/include/couch_eunit.hrl").
+-include_lib("couch/include/couch_db.hrl").
+
+%%%=========================================================================
+%%% Test environment defintions
+%%%=========================================================================
+
+start(App, Apps) ->
+ Ctx = test_util:start_couch(Apps),
+ wait_handlers(App),
+ Ctx.
+
+setup("mocked") ->
--- End diff --
Why not use atoms 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 issue #65: Refactor test suite for httpd endpoints
Posted by janl <gi...@git.apache.org>.
Github user janl commented on the issue:
https://github.com/apache/couchdb-chttpd/pull/65
@iilyak @davisp are you planning to bring this up to master, or can we close this?
---
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: Refactor test suite for httpd endpoin...
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/65#discussion_r37905228
--- Diff: test/chttpd_httpd_handlers_tests.erl ---
@@ -0,0 +1,54 @@
+% 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_httpd_handlers_tests).
+
+-export([handlers/1]).
+
+-include_lib("couch/include/couch_eunit.hrl").
+
+handlers(url_handler) ->
+ [
+ {<<"">>, chttpd_misc, handle_welcome_req},
+ {<<"favicon.ico">>, chttpd_misc, handle_favicon_req},
+ {<<"_utils">>, chttpd_misc, handle_utils_dir_req},
+ {<<"_all_dbs">>, chttpd_misc, handle_all_dbs_req},
+ {<<"_active_tasks">>, chttpd_misc, handle_task_status_req},
+ {<<"_node">>, chttpd_misc, handle_node_req},
+ {<<"_reload_query_servers">>, chttpd_misc, handle_reload_query_servers_req},
+ {<<"_replicate">>, chttpd_misc, handle_replicate_req},
+ {<<"_uuids">>, chttpd_misc, handle_uuids_req},
+ {<<"_session">>, chttpd_auth, handle_session_req},
+ {<<"_up">>, chttpd_misc, handle_up_req},
+ {<<"anything">>, chttpd_db, handle_request}
--- End diff --
> I am also thinking rather it would be better to place tests in <app>_httpd_handlers.erl itself.
+1 - this will at least help to not miss the tests if you're going to modify handlers. My point here was about to not fall into situation, when you change/update `<app>_httpd_handlers.erl` and not update the tests (they may eventually stay green after) and imho this should help a bit.
> It would be trivial with elixir macro :-).
I think use elixir for testing is not a bad idea at all. They have quite friendly and powerful tools for that. But not today (:
---
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 #65: Refactor test suite for httpd endpoints
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak closed the pull request at:
https://github.com/apache/couchdb-chttpd/pull/65
---
[GitHub] couchdb-chttpd pull request: Refactor test suite for httpd endpoin...
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/65#discussion_r37904518
--- Diff: test/chttpd_httpd_handlers_tests.erl ---
@@ -0,0 +1,54 @@
+% 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_httpd_handlers_tests).
+
+-export([handlers/1]).
+
+-include_lib("couch/include/couch_eunit.hrl").
+
+handlers(url_handler) ->
+ [
+ {<<"">>, chttpd_misc, handle_welcome_req},
+ {<<"favicon.ico">>, chttpd_misc, handle_favicon_req},
+ {<<"_utils">>, chttpd_misc, handle_utils_dir_req},
+ {<<"_all_dbs">>, chttpd_misc, handle_all_dbs_req},
+ {<<"_active_tasks">>, chttpd_misc, handle_task_status_req},
+ {<<"_node">>, chttpd_misc, handle_node_req},
+ {<<"_reload_query_servers">>, chttpd_misc, handle_reload_query_servers_req},
+ {<<"_replicate">>, chttpd_misc, handle_replicate_req},
+ {<<"_uuids">>, chttpd_misc, handle_uuids_req},
+ {<<"_session">>, chttpd_auth, handle_session_req},
+ {<<"_up">>, chttpd_misc, handle_up_req},
+ {<<"anything">>, chttpd_db, handle_request}
--- End diff --
I'm actually quite like option 5)
---
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: Refactor test suite for httpd endpoin...
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/65#discussion_r37904128
--- Diff: test/chttpd_httpd_handlers_tests.erl ---
@@ -0,0 +1,54 @@
+% 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_httpd_handlers_tests).
+
+-export([handlers/1]).
+
+-include_lib("couch/include/couch_eunit.hrl").
+
+handlers(url_handler) ->
+ [
+ {<<"">>, chttpd_misc, handle_welcome_req},
+ {<<"favicon.ico">>, chttpd_misc, handle_favicon_req},
+ {<<"_utils">>, chttpd_misc, handle_utils_dir_req},
+ {<<"_all_dbs">>, chttpd_misc, handle_all_dbs_req},
+ {<<"_active_tasks">>, chttpd_misc, handle_task_status_req},
+ {<<"_node">>, chttpd_misc, handle_node_req},
+ {<<"_reload_query_servers">>, chttpd_misc, handle_reload_query_servers_req},
+ {<<"_replicate">>, chttpd_misc, handle_replicate_req},
+ {<<"_uuids">>, chttpd_misc, handle_uuids_req},
+ {<<"_session">>, chttpd_auth, handle_session_req},
+ {<<"_up">>, chttpd_misc, handle_up_req},
+ {<<"anything">>, chttpd_db, handle_request}
--- End diff --
5. Is the simplest one
---
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: Refactor test suite for httpd endpoin...
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/65#issuecomment-134462216
This PR depends on https://github.com/apache/couchdb-couch-epi/pull/6
---
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: Refactor test suite for httpd endpoin...
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/65#discussion_r37883279
--- Diff: test/chttpd_httpd_handlers_tests.erl ---
@@ -0,0 +1,54 @@
+% 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_httpd_handlers_tests).
+
+-export([handlers/1]).
+
+-include_lib("couch/include/couch_eunit.hrl").
+
+handlers(url_handler) ->
+ [
+ {<<"">>, chttpd_misc, handle_welcome_req},
+ {<<"favicon.ico">>, chttpd_misc, handle_favicon_req},
+ {<<"_utils">>, chttpd_misc, handle_utils_dir_req},
+ {<<"_all_dbs">>, chttpd_misc, handle_all_dbs_req},
+ {<<"_active_tasks">>, chttpd_misc, handle_task_status_req},
+ {<<"_node">>, chttpd_misc, handle_node_req},
+ {<<"_reload_query_servers">>, chttpd_misc, handle_reload_query_servers_req},
+ {<<"_replicate">>, chttpd_misc, handle_replicate_req},
+ {<<"_uuids">>, chttpd_misc, handle_uuids_req},
+ {<<"_session">>, chttpd_auth, handle_session_req},
+ {<<"_up">>, chttpd_misc, handle_up_req},
+ {<<"anything">>, chttpd_db, handle_request}
--- End diff --
I wonder if there is a way to not have this list duplicate definitions in chttpd_chttpd_handlers...at least for 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: Refactor test suite for httpd endpoin...
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:
https://github.com/apache/couchdb-chttpd/pull/65#discussion_r37904056
--- Diff: test/chttpd_httpd_handlers_tests.erl ---
@@ -0,0 +1,54 @@
+% 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_httpd_handlers_tests).
+
+-export([handlers/1]).
+
+-include_lib("couch/include/couch_eunit.hrl").
+
+handlers(url_handler) ->
+ [
+ {<<"">>, chttpd_misc, handle_welcome_req},
+ {<<"favicon.ico">>, chttpd_misc, handle_favicon_req},
+ {<<"_utils">>, chttpd_misc, handle_utils_dir_req},
+ {<<"_all_dbs">>, chttpd_misc, handle_all_dbs_req},
+ {<<"_active_tasks">>, chttpd_misc, handle_task_status_req},
+ {<<"_node">>, chttpd_misc, handle_node_req},
+ {<<"_reload_query_servers">>, chttpd_misc, handle_reload_query_servers_req},
+ {<<"_replicate">>, chttpd_misc, handle_replicate_req},
+ {<<"_uuids">>, chttpd_misc, handle_uuids_req},
+ {<<"_session">>, chttpd_auth, handle_session_req},
+ {<<"_up">>, chttpd_misc, handle_up_req},
+ {<<"anything">>, chttpd_db, handle_request}
--- End diff --
It would be trivial with elixir macro :-).
On a serious note though few ideas to consider (crazy ones as well).
1. Use https://github.com/richcarl/merl to generate `handlers` clauses
2. Extract the info from beam chunks somehow
3. Use macro somehow (very problematic with erlang text based preprocessor).
4. Define handlers as it is done in tests (performance impact).
5. Require something like `endpoints(db_handler) -> [<<"_view_cleanup">>, <<"compact">>]` in `<app>_httpd_handlers.erl`
---
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: Refactor test suite for httpd endpoin...
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on the pull request:
https://github.com/apache/couchdb-chttpd/pull/65#issuecomment-135040339
I did rework the structure of the PR a little bit.
Now it has mandatory `endpoint/1` function in every `<app>_httpd_handlers.erl` to facilitate introspection.
Let me know what 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.
---