From: Cyril Hrubis <chrubis@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH v3 2/2] testcases/clock_nanosleep01: convert to use new test library API
Date: Thu, 24 Nov 2016 17:48:33 +0100 [thread overview]
Message-ID: <20161124164832.GH14986@rei.lan> (raw)
In-Reply-To: <184a4d332234197dc9c1f61ca24f6f15aea4ee0b.1480002155.git-series.pvorel@suse.cz>
Hi!
> + tst_res(TINFO, "remain time: %ld %ld", rm.tv_sec, rm.tv_nsec);
Looking closer this message should be printed only for the SEND_SIGINT
testcase.
> + /* result check */
> + if (!TEST_RETURN && (elapsed_ms < expect_ms - MAX_MSEC_DIFF
> + || elapsed_ms > expect_ms + MAX_MSEC_DIFF)) {
> + tst_res(TFAIL| TTERRNO, "time range check returned %ld, expected %d, expected errno %s (%d)",
> + TEST_RETURN, tc->exp_ret, tst_strerrno(tc->exp_err), tc->exp_err);
> + return;
> }
Sorry that I missed that in the earlier patches. The actuall tst_res()
message should explain the reason for the failure here. So something as:
tst_res(TFAIL, "The clock_nanosleep() haven't slept correctly, "
"measured %ldms, expected %ldms +- %d", elapsed_ms, expect_ms,
MAX_MSEC_DIFF);
> - cleanup();
> - tst_exit();
> -
> + if (tc->ttype == SEND_SIGINT && !rm.tv_sec && !rm.tv_nsec) {
> + tst_res(TFAIL | TTERRNO, "remain time check returned %ld, expected %d, expected errno %s (%d)",
> + TEST_RETURN, tc->exp_ret, tst_strerrno(tc->exp_err), tc->exp_err);
Here as well, we should note that the timespec wasn't updated with
remaining time. And we should also zero the structure before the
SEND_SIGINT test, since otherwies this will pass even if kernel does not
store the value since it will contain random data from stack.
And if we wanted to be even more pedantic we should also check that the
remaining time is <= sleep time.
> + return;
> + }
> + if (TEST_RETURN != tc->exp_ret) {
> + tst_res(TFAIL | TTERRNO, "returned %ld, expected %d, expected errno: %s (%d)",
> + TEST_RETURN, tc->exp_ret, tst_strerrno(tc->exp_err), tc->exp_err);
> + return;
> + }
> + tst_res(TPASS, "returned %ld", TEST_RETURN);
> }
Otherwise this version looks very good. Just keep in mind to keep the
lines under 80 characters.
--
Cyril Hrubis
chrubis@suse.cz
next prev parent reply other threads:[~2016-11-24 16:48 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-24 16:01 [LTP] [PATCH v3 1/2] lib: move create_sig_proc() into newlib Petr Vorel
2016-11-24 16:01 ` [LTP] [PATCH v3 2/2] testcases/clock_nanosleep01: convert to use new test library API Petr Vorel
2016-11-24 16:48 ` Cyril Hrubis [this message]
2016-11-25 9:05 ` Petr Vorel
2016-11-28 9:32 ` Cyril Hrubis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161124164832.GH14986@rei.lan \
--to=chrubis@suse.cz \
--cc=ltp@lists.linux.it \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox