From e2dcfa63017faf39993a710de81a1ea173b4d395 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Wed, 11 Oct 2017 14:32:28 -0700 Subject: [PATCH 1/5] modules/kvs/test: Fix test log message --- src/modules/kvs/test/cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/kvs/test/cache.c b/src/modules/kvs/test/cache.c index 92458cc5966e..d16251a33d02 100644 --- a/src/modules/kvs/test/cache.c +++ b/src/modules/kvs/test/cache.c @@ -81,7 +81,7 @@ void cache_entry_basic_tests (void) ok ((otmp = cache_entry_get_json (e)) == NULL, "cache_entry_get_json returns NULL, no json set"); ok (cache_entry_get_raw (e, NULL) == NULL, - "cache_entry_get_json returns NULL, no json set"); + "cache_entry_get_raw returns NULL, no data set"); cache_entry_destroy (e); e = NULL; } From a93c3643805d02e833cf385b79c0d39c3cf26dd5 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Wed, 11 Oct 2017 11:18:15 -0700 Subject: [PATCH 2/5] modules/kvs/test: Add commit API testing coverage --- src/modules/kvs/test/commit.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/modules/kvs/test/commit.c b/src/modules/kvs/test/commit.c index 5b912c3eca00..b0396edd06ea 100644 --- a/src/modules/kvs/test/commit.c +++ b/src/modules/kvs/test/commit.c @@ -827,12 +827,17 @@ void commit_process_missing_ref (void) { int ref_error_cb (commit_t *c, const char *ref, void *data) { + /* pick a weird errno */ + errno = ENOTTY; return -1; } int cache_error_cb (commit_t *c, struct cache_entry *hp, void *data) { commit_cleanup_dirty_cache_entry (c, hp); + + /* pick a weird errno */ + errno = EXDEV; return -1; } @@ -885,8 +890,10 @@ void commit_process_error_callbacks (void) { ok (commit_process (c, 1, root_ref) == COMMIT_PROCESS_LOAD_MISSING_REFS, "commit_process returns COMMIT_PROCESS_LOAD_MISSING_REFS"); - ok (commit_iter_missing_refs (c, ref_error_cb, NULL) < 0, - "commit_iter_missing_refs errors on callback error"); + errno = 0; + ok (commit_iter_missing_refs (c, ref_error_cb, NULL) < 0 + && errno == ENOTTY, + "commit_iter_missing_refs errors on callback error & returns correct errno"); /* insert cache entry now, want don't want missing refs on next * commit_process call */ @@ -895,8 +902,10 @@ void commit_process_error_callbacks (void) { ok (commit_process (c, 1, root_ref) == COMMIT_PROCESS_DIRTY_CACHE_ENTRIES, "commit_process returns COMMIT_PROCESS_DIRTY_CACHE_ENTRIES"); - ok (commit_iter_dirty_cache_entries (c, cache_error_cb, NULL) < 0, - "commit_iter_dirty_cache_entries errors on callback error"); + errno = 0; + ok (commit_iter_dirty_cache_entries (c, cache_error_cb, NULL) < 0 + && errno == EXDEV, + "commit_iter_dirty_cache_entries errors on callback error & returns correct errno"); commit_mgr_destroy (cm); cache_destroy (cache); @@ -914,6 +923,8 @@ int cache_error_partway_cb (commit_t *c, struct cache_entry *hp, void *data) if (epd->total_calls > 1) return -1; epd->success_returns++; + /* pick a weird errno */ + errno = EDOM; return 0; } @@ -972,8 +983,10 @@ void commit_process_error_callbacks_partway (void) { ok (commit_process (c, 1, root_ref) == COMMIT_PROCESS_DIRTY_CACHE_ENTRIES, "commit_process returns COMMIT_PROCESS_DIRTY_CACHE_ENTRIES"); - ok (commit_iter_dirty_cache_entries (c, cache_error_partway_cb, &epd) < 0, - "commit_iter_dirty_cache_entries errors on callback error"); + errno = 0; + ok (commit_iter_dirty_cache_entries (c, cache_error_partway_cb, &epd) < 0 + && errno == EDOM, + "commit_iter_dirty_cache_entries errors on callback error & returns correct errno"); ok (epd.total_calls == 2, "correct number of total calls to dirty cache callback"); From 9875f2df71582935e7efd598deceb50c25d29f8f Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Wed, 11 Oct 2017 23:28:41 -0700 Subject: [PATCH 3/5] modules/kvs/test: Add cache tests Create waiter tests to work with raw data cache entries. --- src/modules/kvs/test/cache.c | 73 +++++++++++++++++++++++++++++++++++- 1 file changed, 71 insertions(+), 2 deletions(-) diff --git a/src/modules/kvs/test/cache.c b/src/modules/kvs/test/cache.c index d16251a33d02..729b8d602b15 100644 --- a/src/modules/kvs/test/cache.c +++ b/src/modules/kvs/test/cache.c @@ -332,7 +332,7 @@ void cache_entry_raw_tests (void) e = NULL; } -void waiter_tests (void) +void waiter_json_tests (void) { struct cache_entry *e; json_t *o; @@ -401,6 +401,74 @@ void waiter_tests (void) cache_entry_destroy (e); /* destroys o */ } +void waiter_raw_tests (void) +{ + struct cache_entry *e; + char *data; + wait_t *w; + int count; + + /* Test cache entry waiters. + * N.B. waiter is destroyed when run. + */ + count = 0; + ok ((w = wait_create (wait_cb, &count)) != NULL, + "wait_create works"); + ok ((e = cache_entry_create ()) != NULL, + "cache_entry_create created empty object"); + ok (cache_entry_get_valid (e) == false, + "cache entry invalid, adding waiter"); + ok (cache_entry_clear_dirty (e) < 0, + "cache_entry_clear_dirty returns error, b/c no object set"); + ok (cache_entry_force_clear_dirty (e) < 0, + "cache_entry_force_clear_dirty returns error, b/c no object set"); + ok (cache_entry_wait_valid (e, w) == 0, + "cache_entry_wait_valid success"); + data = strdup ("abcd"); + ok (cache_entry_set_raw (e, data, 4) == 0, + "cache_entry_set_raw success"); + ok (cache_entry_get_valid (e) == true, + "cache entry set valid with one waiter"); + ok (count == 1, + "waiter callback ran"); + + count = 0; + ok ((w = wait_create (wait_cb, &count)) != NULL, + "wait_create works"); + ok (cache_entry_set_dirty (e, true) == 0, + "cache_entry_set_dirty success"); + ok (cache_entry_get_dirty (e) == true, + "cache entry set dirty, adding waiter"); + ok (cache_entry_wait_notdirty (e, w) == 0, + "cache_entry_wait_notdirty success"); + ok (cache_entry_clear_dirty (e) == 1, + "cache_entry_clear_dirty returns 1, b/c of a waiter"); + ok (cache_entry_set_dirty (e, false) == 0, + "cache_entry_set_dirty success"); + ok (cache_entry_get_dirty (e) == false, + "cache entry set not dirty with one waiter"); + ok (count == 1, + "waiter callback ran"); + + count = 0; + ok ((w = wait_create (wait_cb, &count)) != NULL, + "wait_create works"); + ok (cache_entry_set_dirty (e, true) == 0, + "cache_entry_set_dirty success"); + ok (cache_entry_get_dirty (e) == true, + "cache entry set dirty, adding waiter"); + ok (cache_entry_wait_notdirty (e, w) == 0, + "cache_entry_wait_notdirty success"); + ok (cache_entry_force_clear_dirty (e) == 0, + "cache_entry_clear_dirty returns 0 w/ waiter"); + ok (cache_entry_get_dirty (e) == false, + "cache entry set not dirty with one waiter"); + ok (count == 0, + "waiter callback not called on force clear dirty"); + + cache_entry_destroy (e); /* destroys o */ +} + void cache_remove_entry_tests (void) { struct cache *cache; @@ -600,7 +668,8 @@ int main (int argc, char *argv[]) cache_entry_basic_tests (); cache_entry_json_tests (); cache_entry_raw_tests (); - waiter_tests (); + waiter_json_tests (); + waiter_raw_tests (); cache_expiration_tests (); cache_remove_entry_tests (); From 03f121b6bc80817988b2b40fadb7dde8552c7d65 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Fri, 6 Oct 2017 10:49:19 -0700 Subject: [PATCH 4/5] modules/kvs: Fix errnum return corner case Fix corner case in which error from commit wasn't passed to cleanup path correctly. --- src/modules/kvs/kvs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/kvs/kvs.c b/src/modules/kvs/kvs.c index 56616a78e599..4fd623e19c38 100644 --- a/src/modules/kvs/kvs.c +++ b/src/modules/kvs/kvs.c @@ -516,7 +516,7 @@ static void commit_apply (commit_t *c) int errnum = 0; commit_process_t ret; - if (commit_get_aux_errnum (c)) + if ((errnum = commit_get_aux_errnum (c))) goto done; if ((ret = commit_process (c, From 0e4f97f04c021d9d320fa8ca7ebbcdbd9e409106 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Tue, 10 Oct 2017 08:47:58 -0700 Subject: [PATCH 5/5] modules/kvs: Rename commit callback types Per style guidelines in RFC 7, rename function typedefs with '_f' at the end of the name. More specificlly, rename commit_ref_cb to commit_ref_f and commit_cache_entry_cb to commit_cache_entry_f. --- src/modules/kvs/commit.c | 4 ++-- src/modules/kvs/commit.h | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/modules/kvs/commit.c b/src/modules/kvs/commit.c index 20d016ef5864..2049752f0bec 100644 --- a/src/modules/kvs/commit.c +++ b/src/modules/kvs/commit.c @@ -684,7 +684,7 @@ commit_process_t commit_process (commit_t *c, return COMMIT_PROCESS_DIRTY_CACHE_ENTRIES; } -int commit_iter_missing_refs (commit_t *c, commit_ref_cb cb, void *data) +int commit_iter_missing_refs (commit_t *c, commit_ref_f cb, void *data) { const char *ref; int saved_errno, rc = 0; @@ -712,7 +712,7 @@ int commit_iter_missing_refs (commit_t *c, commit_ref_cb cb, void *data) } int commit_iter_dirty_cache_entries (commit_t *c, - commit_cache_entry_cb cb, + commit_cache_entry_f cb, void *data) { struct cache_entry *hp; diff --git a/src/modules/kvs/commit.h b/src/modules/kvs/commit.h index b2bde6ac7b0e..04d4d03da377 100644 --- a/src/modules/kvs/commit.h +++ b/src/modules/kvs/commit.h @@ -22,9 +22,9 @@ typedef enum { * commit_t API */ -typedef int (*commit_ref_cb)(commit_t *c, const char *ref, void *data); +typedef int (*commit_ref_f)(commit_t *c, const char *ref, void *data); -typedef int (*commit_cache_entry_cb)(commit_t *c, +typedef int (*commit_cache_entry_f)(commit_t *c, struct cache_entry *hp, void *data); @@ -75,7 +75,7 @@ commit_process_t commit_process (commit_t *c, * * return -1 in callback to break iteration */ -int commit_iter_missing_refs (commit_t *c, commit_ref_cb cb, void *data); +int commit_iter_missing_refs (commit_t *c, commit_ref_f cb, void *data); /* on commit stall, iterate through all dirty cache entries that need * to be pushed to the content store. @@ -83,7 +83,7 @@ int commit_iter_missing_refs (commit_t *c, commit_ref_cb cb, void *data); * return -1 in callback to break iteration */ int commit_iter_dirty_cache_entries (commit_t *c, - commit_cache_entry_cb cb, + commit_cache_entry_f cb, void *data); /* convenience function for cleaning up a dirty cache entry that was