All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Latypov <dlatypov@google.com>
To: brendanhiggins@google.com, davidgow@google.com
Cc: linux-kernel@vger.kernel.org, kunit-dev@googlegroups.com,
	linux-kselftest@vger.kernel.org, skhan@linuxfoundation.org,
	Daniel Latypov <dlatypov@google.com>
Subject: [PATCH v3 3/4] kunit: tool: actually track how long it took to run tests
Date: Wed, 29 Sep 2021 12:54:35 -0700	[thread overview]
Message-ID: <20210929195436.1405996-4-dlatypov@google.com> (raw)
In-Reply-To: <20210929195436.1405996-1-dlatypov@google.com>

This is a long standing bug in kunit tool.
Since these files were added, run_kernel() has always yielded lines.

That means, the call to run_kernel() returns before the kernel finishes
executing tests, potentially before a single line of output is even
produced.

So code like this
  time_start = time.time()
  result = linux.run_kernel(...)
  time_end = time.time()

would only measure the time taken for python to give back the generator
object.

From a caller's perspective, the only way to know the kernel has exited
is for us to consume all the output from the `result` generator object.
Alternatively, we could change run_kernel() to try and do its own book
keeping and return the total time, but that doesn't seem worth it.

This change makes us record `time_end` after we're doing parsing all the
output (which should mean we've consumed all of it, or errored out).
That means we're including in the parsing time as well, but that should
be quite small, and it's better than claiming it took 0s to run tests.

Let's use this as an example:
$ ./tools/testing/kunit/kunit.py run --kunitconfig=lib/kunit example

Before:
Elapsed time: 7.684s total, 0.001s configuring, 4.692s building, 0.000s running

After:
Elapsed time: 6.283s total, 0.001s configuring, 3.202s building, 3.079s running

Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
 tools/testing/kunit/kunit.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index 31eec9f6ecc3..5e717594df5b 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -101,12 +101,14 @@ def exec_tests(linux: kunit_kernel.LinuxSourceTree, request: KunitExecRequest,
 		filter_glob=request.filter_glob,
 		build_dir=request.build_dir)
 
+	result = parse_tests(parse_request, run_result)
+
+	# run_kernel() doesn't block on the kernel exiting.
+	# That only happens after we get the last line of output from `run_result`.
+	# So exec_time here actually contains parsing + execution time, which is fine.
 	test_end = time.time()
 	exec_time = test_end - test_start
 
-	# Named tuples are immutable, so we rebuild them here manually
-	result = parse_tests(parse_request, run_result)
-
 	return KunitResult(status=result.status, result=result.result, elapsed_time=exec_time)
 
 def parse_tests(request: KunitParseRequest, input_data: Iterable[str]) -> KunitResult:
-- 
2.33.0.685.g46640cef36-goog


  parent reply	other threads:[~2021-09-29 19:54 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-29 19:54 [PATCH v3 0/3] kunit: allow running test suites/cases individually Daniel Latypov
2021-09-29 19:54 ` [PATCH v3 1/4] kunit: add 'kunit.action' param to allow listing out tests Daniel Latypov
2021-09-29 19:54 ` [PATCH v3 2/4] kunit: tool: factor exec + parse steps into a function Daniel Latypov
2021-09-29 19:54 ` Daniel Latypov [this message]
2021-09-30  2:29   ` [PATCH v3 3/4] kunit: tool: actually track how long it took to run tests David Gow
2021-09-29 19:54 ` [PATCH v3 4/4] kunit: tool: support running each suite/test separately Daniel Latypov
2021-09-30  2:27   ` David Gow
2021-09-30 16:30     ` Daniel Latypov

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=20210929195436.1405996-4-dlatypov@google.com \
    --to=dlatypov@google.com \
    --cc=brendanhiggins@google.com \
    --cc=davidgow@google.com \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=skhan@linuxfoundation.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.