Skip to content
This repository was archived by the owner on May 25, 2021. It is now read-only.

Refactor test suite for httpd endpoints#65

Closed
iilyak wants to merge 5 commits intoapache:masterfrom
cloudant:2789-httpd_endpoints_testsuite
Closed

Refactor test suite for httpd endpoints#65
iilyak wants to merge 5 commits intoapache:masterfrom
cloudant:2789-httpd_endpoints_testsuite

Conversation

@iilyak
Copy link
Copy Markdown
Contributor

@iilyak iilyak commented Aug 25, 2015

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

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
@iilyak
Copy link
Copy Markdown
Contributor Author

iilyak commented Aug 25, 2015

This PR depends on apache/couchdb-couch-epi#6

Comment thread src/chttpd_httpd_handlers_test_util.erl Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use atoms here?

Comment thread test/chttpd_httpd_handlers_tests.erl Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there is a way to not have this list duplicate definitions in chttpd_chttpd_handlers...at least for handlers.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Number 5) Is the simplest one

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a side note. I am also thinking rather it would be better to place tests in <app>_httpd_handlers.erl itself.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm actually quite like option 5)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am also thinking rather it would be better to place tests in _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 (:

@iilyak
Copy link
Copy Markdown
Contributor Author

iilyak commented Aug 26, 2015

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.

@kxepal
Copy link
Copy Markdown
Member

kxepal commented Aug 26, 2015

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 (:

@davisp davisp deleted the 2789-httpd_endpoints_testsuite branch October 22, 2015 06:36
@davisp davisp restored the 2789-httpd_endpoints_testsuite branch December 8, 2015 17:48
@janl
Copy link
Copy Markdown
Member

janl commented Jun 3, 2017

@iilyak @davisp are you planning to bring this up to master, or can we close this?

@iilyak
Copy link
Copy Markdown
Contributor Author

iilyak commented Sep 12, 2018

stale PR

@iilyak iilyak closed this Sep 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants