You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by jaydoane <gi...@git.apache.org> on 2015/11/05 09:07:19 UTC
[GitHub] couchdb-couch pull request: Run tests with only the couch_db_plugi...
GitHub user jaydoane opened a pull request:
https://github.com/apache/couchdb-couch/pull/125
Run tests with only the couch_db_plugin_test couch_epi plugin
Save current couch_epi application environment plugins in context during test setup, and restore during teardown.
COUCHDB-2868
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cloudant/couchdb-couch 2868-fix-couch_db_plugin_tests-downstream
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-couch/pull/125.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 #125
----
commit 326b3061e8eff8e188638a0ea4efc5fda9d98d7f
Author: Jay Doane <ja...@gmail.com>
Date: 2015-11-05T08:03:32Z
Run tests with only the couch_db_plugin_test couch_epi plugin
Save current couch_epi application environment plugins in context during test setup, and restore during teardown.
COUCHDB-2868
----
---
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-couch pull request: Run tests with only the couch_db_plugi...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/couchdb-couch/pull/125
---
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-couch pull request: Run tests with only the couch_db_plugi...
Posted by jaydoane <gi...@git.apache.org>.
Github user jaydoane commented on the pull request:
https://github.com/apache/couchdb-couch/pull/125#issuecomment-153984529
@iilyak, does this look reasonable 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-couch pull request: Run tests with only the couch_db_plugi...
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/125#discussion_r44017552
--- Diff: test/couch_db_plugin_tests.erl ---
@@ -48,24 +48,18 @@ notify(_, _, _) -> ok.
start_epi() ->
application:load(couch_epi),
- Plugins = application:get_env(couch_epi, plugins, []),
- ok = application:set_env(couch_epi, plugins, append_if_missing(Plugins, ?MODULE)),
- ok = application:start(couch_epi).
-
-append_if_missing(List, Value) ->
- case lists:member(Value, List) of
- true -> List;
- false -> [Value | List]
- end.
+ SavedPlugins = application:get_env(couch_epi, plugins, []),
+ ok = application:set_env(couch_epi, plugins, [?MODULE]),
+ ok = application:start(couch_epi),
+ SavedPlugins.
setup() ->
error_logger:tty(false),
- start_epi(),
- #ctx{handle = couch_epi:get_handle(couch_db)}.
+ SavedPlugins = start_epi(),
+ #ctx{handle = couch_epi:get_handle(couch_db), saved_plugins=SavedPlugins}.
-teardown(#ctx{}) ->
- Plugins = application:get_env(couch_epi, plugins, []),
- application:set_env(couch_epi, plugins, Plugins -- [?MODULE]),
+teardown(#ctx{saved_plugins=SavedPlugins}) ->
--- End diff --
spaces
---
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-couch pull request: Run tests with only the couch_db_plugi...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/125#issuecomment-154442209
@jaydoane ok with separate issue. +1
---
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-couch pull request: Run tests with only the couch_db_plugi...
Posted by jaydoane <gi...@git.apache.org>.
Github user jaydoane commented on the pull request:
https://github.com/apache/couchdb-couch/pull/125#issuecomment-154204185
The latest changes clean the setup/teardown code considerably, in addition to speeding up the test suite.
@kxepal, may I suggest that if we want an example test showing epi plugin collisions/interference, that we create a separate issue for that? I'm happy to tackle that if we can get this resolved...
---
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-couch pull request: Run tests with only the couch_db_plugi...
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/125#issuecomment-154128121
I think it worth to add some test that cause failure for that case before fix it. Because, we didn't have such problem now and we eventually may break this again without any notice.
---
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-couch pull request: Run tests with only the couch_db_plugi...
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/125#discussion_r44017514
--- Diff: test/couch_db_plugin_tests.erl ---
@@ -48,24 +48,18 @@ notify(_, _, _) -> ok.
start_epi() ->
application:load(couch_epi),
- Plugins = application:get_env(couch_epi, plugins, []),
- ok = application:set_env(couch_epi, plugins, append_if_missing(Plugins, ?MODULE)),
- ok = application:start(couch_epi).
-
-append_if_missing(List, Value) ->
- case lists:member(Value, List) of
- true -> List;
- false -> [Value | List]
- end.
+ SavedPlugins = application:get_env(couch_epi, plugins, []),
+ ok = application:set_env(couch_epi, plugins, [?MODULE]),
+ ok = application:start(couch_epi),
+ SavedPlugins.
setup() ->
error_logger:tty(false),
- start_epi(),
- #ctx{handle = couch_epi:get_handle(couch_db)}.
+ SavedPlugins = start_epi(),
+ #ctx{handle = couch_epi:get_handle(couch_db), saved_plugins=SavedPlugins}.
--- End diff --
Please add space before and after `=` sign.
---
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-couch pull request: Run tests with only the couch_db_plugi...
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on the pull request:
https://github.com/apache/couchdb-couch/pull/125#issuecomment-154422349
+1
---
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-couch pull request: Run tests with only the couch_db_plugi...
Posted by jaydoane <gi...@git.apache.org>.
Github user jaydoane commented on the pull request:
https://github.com/apache/couchdb-couch/pull/125#issuecomment-154158219
@iilyak, I agree with your last comment, as it seems a bit strange to save the plugin state while tests are running.
@kxepal, the reason check_is_admin_no_match was failing downstream was because -- in addition to loading the couch_db_pluging_tests plugin that was meant to handle that case -- all the couch_epi plugins specified in Cloudant's couch_epi.config were being loaded, and one of them was inadvertently handling (and breaking) the test case. I think any time a unit test pulls in configuration data that it doesn't completely specify, we run the risk of it breaking something downstream. I could certainly create another plugin that simulates what was happening, but I don't think it will really help prevent this sort of thing in the future. By loading *only* the plugins we want to test, we force test behavior downstream to be the same. Does that make sense?
---
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-couch pull request: Run tests with only the couch_db_plugi...
Posted by iilyak <gi...@git.apache.org>.
Github user iilyak commented on the pull request:
https://github.com/apache/couchdb-couch/pull/125#issuecomment-154079245
Maybe we should stop and unload couch_epi in teardown instead. Since we load and start it in setup. In this case there will be no need to remember current list of configured plugins.
---
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.
---