[{"id":28585,"web_url":"https://patchwork.libcamera.org/comment/28585/","msgid":"<875xzkb6t1.fsf@redhat.com>","date":"2024-01-23T11:58:02","subject":"Re: [PATCH v2 09/12] test: timer-thread: Move timer start from\n\twrong thread to separate test","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Laurent Pinchart writes:\n\n> Starting a timer from the wrong thread is expected to fail, and we test\n> this in the timer-thread unit test. This is however not something that a\n> caller is allowed to do, and libcamera will get assertion failures to\n> catch this invalid usage. The unit test will then fail.\n>\n> To prepare for this, split the unit test in two, with a test that is\n> expected by meson to succeed, and one that is expected to fail. The\n> assertion will then cause an expected failure, making the test suite\n> succeed.\n>\n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Milan Zamazal \n\n> ---\n> Changes since v1:\n>\n> - Improve comment explaining why the test is expected to fail\n> ---\n> test/meson.build | 9 ++--\n> test/timer-fail.cpp | 103 ++++++++++++++++++++++++++++++++++++++++++\n> test/timer-thread.cpp | 22 ---------\n> 3 files changed, 109 insertions(+), 25 deletions(-)\n> create mode 100644 test/timer-fail.cpp\n>\n> diff --git a/test/meson.build b/test/meson.build\n> index 189e1428485a..8b6057d4e800 100644\n> --- a/test/meson.build\n> +++ b/test/meson.build\n> @@ -69,6 +69,7 @@ internal_tests = [\n> {'name': 'signal-threads', 'sources': ['signal-threads.cpp']},\n> {'name': 'threads', 'sources': 'threads.cpp', 'dependencies': [libthreads]},\n> {'name': 'timer', 'sources': ['timer.cpp']},\n> + {'name': 'timer-fail', 'sources': ['timer-fail.cpp'], 'should_fail': true},\n> {'name': 'timer-thread', 'sources': ['timer-thread.cpp']},\n> {'name': 'unique-fd', 'sources': ['unique-fd.cpp']},\n> {'name': 'utils', 'sources': ['utils.cpp']},\n> @@ -91,7 +92,7 @@ foreach test : public_tests\n> link_with : test_libraries,\n> include_directories : test_includes_public)\n> \n> - test(test['name'], exe)\n> + test(test['name'], exe, should_fail : test.get('should_fail', false))\n> endforeach\n> \n> foreach test : internal_tests\n> @@ -105,7 +106,7 @@ foreach test : internal_tests\n> link_with : test_libraries,\n> include_directories : test_includes_internal)\n> \n> - test(test['name'], exe)\n> + test(test['name'], exe, should_fail : test.get('should_fail', false))\n> endforeach\n> \n> foreach test : internal_non_parallel_tests\n> @@ -119,5 +120,7 @@ foreach test : internal_non_parallel_tests\n> link_with : test_libraries,\n> include_directories : test_includes_internal)\n> \n> - test(test['name'], exe, is_parallel : false)\n> + test(test['name'], exe,\n> + is_parallel : false,\n> + should_fail : test.get('should_fail', false))\n> endforeach\n> diff --git a/test/timer-fail.cpp b/test/timer-fail.cpp\n> new file mode 100644\n> index 000000000000..2c622ca3410d\n> --- /dev/null\n> +++ b/test/timer-fail.cpp\n> @@ -0,0 +1,103 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2024, Ideas on Board Oy\n> + *\n> + * timer-fail.cpp - Threaded timer failure test\n> + */\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \"test.h\"\n> +\n> +using namespace libcamera;\n> +using namespace std;\n> +using namespace std::chrono_literals;\n> +\n> +class TimeoutHandler : public Object\n> +{\n> +public:\n> +\tTimeoutHandler()\n> +\t\t: timer_(this), timeout_(false)\n> +\t{\n> +\t\ttimer_.timeout.connect(this, &TimeoutHandler::timeoutHandler);\n> +\t}\n> +\n> +\tvoid start()\n> +\t{\n> +\t\ttimer_.start(100ms);\n> +\t}\n> +\n> +\tbool timeout() const\n> +\t{\n> +\t\treturn timeout_;\n> +\t}\n> +\n> +private:\n> +\tvoid timeoutHandler()\n> +\t{\n> +\t\ttimeout_ = true;\n> +\t}\n> +\n> +\tTimer timer_;\n> +\tbool timeout_;\n> +};\n> +\n> +class TimerFailTest : public Test\n> +{\n> +protected:\n> +\tint init()\n> +\t{\n> +\t\tthread_.start();\n> +\t\ttimeout_.moveToThread(&thread_);\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tint run()\n> +\t{\n> +\t\t/*\n> +\t\t * Test that the forbidden operation of starting the timer from\n> +\t\t * another thread results in a failure. We need to interrupt the\n> +\t\t * event dispatcher to make sure we don't succeed simply because\n> +\t\t * the event dispatcher hasn't noticed the timer restart.\n> +\t\t */\n> +\t\ttimeout_.start();\n> +\t\tthread_.eventDispatcher()->interrupt();\n> +\n> +\t\tthis_thread::sleep_for(chrono::milliseconds(200));\n> +\n> +\t\t/*\n> +\t\t * The wrong start() call should result in an assertion in debug\n> +\t\t * builds, and a timeout in release builds. The test is\n> +\t\t * therefore marked in meson.build as expected to fail. We need\n> +\t\t * to return TestPass in the unexpected (usually known as\n> +\t\t * \"fail\") case, and TestFail otherwise.\n> +\t\t */\n> +\t\tif (timeout_.timeout()) {\n> +\t\t\tcout << \"Timer start from wrong thread succeeded unexpectedly\"\n> +\t\t\t << endl;\n> +\t\t\treturn TestPass;\n> +\t\t}\n> +\n> +\t\treturn TestFail;\n> +\t}\n> +\n> +\tvoid cleanup()\n> +\t{\n> +\t\t/* Must stop thread before destroying timeout. */\n> +\t\tthread_.exit(0);\n> +\t\tthread_.wait();\n> +\t}\n> +\n> +private:\n> +\tTimeoutHandler timeout_;\n> +\tThread thread_;\n> +};\n> +\n> +TEST_REGISTER(TimerFailTest)\n> diff --git a/test/timer-thread.cpp b/test/timer-thread.cpp\n> index 0bcd0d8ce194..4caf4e33b2ba 100644\n> --- a/test/timer-thread.cpp\n> +++ b/test/timer-thread.cpp\n> @@ -29,12 +29,6 @@ public:\n> \t\ttimer_.start(100ms);\n> \t}\n> \n> -\tvoid restart()\n> -\t{\n> -\t\ttimeout_ = false;\n> -\t\ttimer_.start(100ms);\n> -\t}\n> -\n> \tbool timeout() const\n> \t{\n> \t\treturn timeout_;\n> @@ -74,22 +68,6 @@ protected:\n> \t\t\treturn TestFail;\n> \t\t}\n> \n> -\t\t/*\n> -\t\t * Test that starting the timer from another thread fails. We\n> -\t\t * need to interrupt the event dispatcher to make sure we don't\n> -\t\t * succeed simply because the event dispatcher hasn't noticed\n> -\t\t * the timer restart.\n> -\t\t */\n> -\t\ttimeout_.restart();\n> -\t\tthread_.eventDispatcher()->interrupt();\n> -\n> -\t\tthis_thread::sleep_for(chrono::milliseconds(200));\n> -\n> -\t\tif (timeout_.timeout()) {\n> -\t\t\tcout << \"Timer restart test failed\" << endl;\n> -\t\t\treturn TestFail;\n> -\t\t}\n> -\n> \t\treturn TestPass;\n> \t}","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7BC29BDC71\n\tfor ;\n\tTue, 23 Jan 2024 11:58:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7C61F62947;\n\tTue, 23 Jan 2024 12:58:10 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E921F628E9\n\tfor ;\n\tTue, 23 Jan 2024 12:58:08 +0100 (CET)","from mail-wm1-f70.google.com (mail-wm1-f70.google.com\n\t[209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-524-Buhc3t9EO6q4D8BBq6yr3g-1; Tue, 23 Jan 2024 06:58:06 -0500","by mail-wm1-f70.google.com with SMTP id\n\t5b1f17b1804b1-40eb6ce1ddaso5747395e9.1\n\tfor ;\n\tTue, 23 Jan 2024 03:58:06 -0800 (PST)","from nuthatch (nat-pool-brq-t.redhat.com. [213.175.37.10])\n\tby smtp.gmail.com with ESMTPSA id\n\tbe13-20020a05600c1e8d00b0040e96a98762sm16483340wmb.17.2024.01.23.03.58.03\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 23 Jan 2024 03:58:03 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"LIW0LigP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1706011088;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=g50lo2+xrN23tQT1lgtNyTJXcGc59g84BxwjB0pfkIU=;\n\tb=LIW0LigPBItJbAyUsY6JNN3nMqO2uCNqZzSLqlWwTTnsIMAGbARa3xwoutuSI4tTp+FMKh\n\tPrphow8o5+4M2PWQh2lwEEOt8phBi/8A2Yojy3orB7Iy1b9R9fu5XNJL0wsK19uH1c2e4S\n\tgsV1vwStlJ5CiLuHFHVNLNSIddEfE+k=","X-MC-Unique":"Buhc3t9EO6q4D8BBq6yr3g-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1706011084; x=1706615884;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=g50lo2+xrN23tQT1lgtNyTJXcGc59g84BxwjB0pfkIU=;\n\tb=OEs0qwKRL6xF1k6Rhzw2sksFTdU/QmGXy+l5f8rcx16kMybtpnNQd8mik1tU/Yd6mi\n\tHOBh1pMP2R5R+4yExHX26Otmy5F0QfUSkI4/ZH4rgWl0mSvOQpyf1zIl6bBzD9Xdm0nB\n\t8uUtSLlzhO5dnFjFhLsDjVooDXQxEXD9J4EGelY8s0GmigHc8NDQLeea7HmkjtdmhjRv\n\tIJ5i8VZgc32wARLMaTcp5cfDW2n5ciXUKdLNoeEJCVfgnK4Pd/2GgOPjKQmhpEduXAo8\n\t+dpKBF5Mel4IzqAxHLneeD9pWPQZ5p3pEUW6Y5q//ePWhn59Iy+/LcQHmY+lCIMDd7VS\n\tGjTg==","X-Gm-Message-State":"AOJu0Yz/Z6uFgJUUYWw8g4A7AOQ+4zqxmVzLi76qCoJLiKS+PLLVVkh2\n\tcJAoMjMUEFq6UwpZaI9ViGGTe+vhMM6/+cqLjq+GJufgNR4Zh3ma8UXppMpbDBcL0sAsEuonT9s\n\tXEoSdRs3DmUbIr1O9Dm9nSUdwL34xDiDTju8b+wFJLrI0k/81242PQYtxobbdPYYOejvkG5CWZ1\n\tBoIxCEbJNelFgHMxptjfsZNN08vgkungd01FM9EQYEr0YTFBqAoeTgFVo=","X-Received":["by 2002:a05:600c:14b:b0:40e:8cc2:748b with SMTP id\n\tw11-20020a05600c014b00b0040e8cc2748bmr239969wmm.299.1706011084146; \n\tTue, 23 Jan 2024 03:58:04 -0800 (PST)","by 2002:a05:600c:14b:b0:40e:8cc2:748b with SMTP id\n\tw11-20020a05600c014b00b0040e8cc2748bmr239965wmm.299.1706011083733; \n\tTue, 23 Jan 2024 03:58:03 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IFozJ3M0rgnCBJqmx2r12o+q69O+p80Z2ZPY1Xy0c0oc7iWRjKxBbZmsNstcX+8qwaHjYuIRw==","From":"Milan Zamazal ","To":"Laurent Pinchart ","Subject":"Re: [PATCH v2 09/12] test: timer-thread: Move timer start from\n\twrong thread to separate test","In-Reply-To":"<20240123011249.22716-10-laurent.pinchart@ideasonboard.com>\n\t(Laurent Pinchart's message of \"Tue, 23 Jan 2024 03:12:46 +0200\")","References":"<20240123011249.22716-1-laurent.pinchart@ideasonboard.com>\n\t<20240123011249.22716-10-laurent.pinchart@ideasonboard.com>","Date":"Tue, 23 Jan 2024 12:58:02 +0100","Message-ID":"<875xzkb6t1.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]