Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

master: Issue 390: Clarify documentation of qb_loop_timer_expire_time… #391

Merged
merged 1 commit into from
Apr 29, 2020

Conversation

cmurphycode
Copy link
Contributor

…_get and provide new function to return previously documented behavior

As discussed in #390. I'm not delighted with how this turned out (lots of shared code) but here goes anyway. It compiles...sorry, I'm not familiar with your testing.

Thanks!

@knet-ci-bot
Copy link

Can one of the admins verify this patch?

@chrissie-c
Copy link
Contributor

Thanks. I also propose this addition to the unit tests

diff --git a/tests/check_loop.c b/tests/check_loop.c
index ac9bbbf..81cc2ba 100644
--- a/tests/check_loop.c
+++ b/tests/check_loop.c
@@ -348,6 +348,22 @@ static Suite *loop_job_suite(void)
  *  Timers
  */
 static qb_loop_timer_handle test_th;
+static qb_loop_timer_handle test_th2;
+
+static void check_time_left(void *data)
+{
+	qb_loop_t *l = (qb_loop_t *)data;
+
+	/* NOTE: We are checking the 'stop_loop' timer here, not our own */
+	uint64_t abs_time = qb_loop_timer_expire_time_get(l, test_th);
+	uint64_t rel_time = qb_loop_timer_expire_time_remaining(l, test_th);
+
+	ck_assert(abs_time > 0ULL);
+	ck_assert(rel_time > 0ULL);
+	ck_assert(abs_time >  rel_time);
+	ck_assert(rel_time <= 60*QB_TIME_NS_IN_MSEC);
+}
+
 
 START_TEST(test_loop_timer_input)
 {
@@ -409,6 +425,9 @@ START_TEST(test_loop_timer_basic)
 	res = qb_loop_timer_add(l, QB_LOOP_LOW, 7*QB_TIME_NS_IN_MSEC, l, reset_one_shot_tmo, &reset_th);
 	ck_assert_int_eq(res, 0);
 
+	res = qb_loop_timer_add(l, QB_LOOP_HIGH, 20*QB_TIME_NS_IN_MSEC, l, check_time_left, &test_th2);
+	ck_assert_int_eq(res, 0);
+
 	res = qb_loop_timer_add(l, QB_LOOP_LOW, 60*QB_TIME_NS_IN_MSEC, l, job_stop, &test_th);
 	ck_assert_int_eq(res, 0);
 

…_get and provide new function to return previously documented behavior

Includes unit test addition by chrissie-c
@cmurphycode
Copy link
Contributor Author

@chrissie-c Thanks for the test! I added it and amended the original commit.

@chrissie-c
Copy link
Contributor

retest this please

1 similar comment
@chrissie-c
Copy link
Contributor

retest this please

@chrissie-c chrissie-c merged commit 08806c5 into ClusterLabs:master Apr 29, 2020
@chrissie-c
Copy link
Contributor

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants