From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id D88D04590C; Thu, 5 Sep 2024 11:23:43 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C218440264; Thu, 5 Sep 2024 11:23:43 +0200 (CEST) Received: from mail-lj1-f182.google.com (mail-lj1-f182.google.com [209.85.208.182]) by mails.dpdk.org (Postfix) with ESMTP id 4DA964025C for ; Thu, 5 Sep 2024 11:23:42 +0200 (CEST) Received: by mail-lj1-f182.google.com with SMTP id 38308e7fff4ca-2f406034874so6223451fa.1 for ; Thu, 05 Sep 2024 02:23:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1725528222; x=1726133022; darn=dpdk.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=ad6SE6xJRDckZzKEbPKRJC9BK1au62Z00TK6sKe4iUA=; b=qlGCXU5tV4gYQjmmwodCxKehRjlwmOcQH9r/4immTxriw4g8DdChKo8lWXvPVpOYlp p3X891YxiqpLS/PQWuVlVUWSitNZy1Jr6CftW4bdK++lvzczomQ25oEDNp7FSbht3jEw 2Us2/g1bxPJD+aChgFIqAtnVGeW1tXxr96HXEZIwI+meaBA4Wjt5HJyC+qFegnanbwun 9g308F6Fk73X/FRUQQBzwKu1onHWfISfx7BWB9T4TjnShrn5h58YISYSrOK+XiigMl/T zwo4dJDh3t7hu22aVp6hysJyBu94JydRGCRy7KaRIafzYgjge3rj7DQ+r6BXYs4et7dN /kSg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1725528222; x=1726133022; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=ad6SE6xJRDckZzKEbPKRJC9BK1au62Z00TK6sKe4iUA=; b=oOcvbFHSgmuoreJ73WF5TlWD3L6XrPqQDSeGmuv7vzSbXIVLtINX5Nn6Ct4oRetmH5 XX8fGAUHHNet5rWKBSBrqpOwTM/4J2kughH0YWwHTmyl6WYk59mMIHv7bVBMkqb9CIVL FF8krYPjfyJU9Kp6InMeoIW20Qj0AE4j9amYEDzks9dMD3wkrHVmW9n/IwKL2AKbQemZ NcyB9q0jZtlgiBCV65TyHgmIDPreuAM7c0vO2VncjJKtynT+Wj28Vfa8W9CBr/U2TBXs OU0/8iawrv40CrxKu2DldGz7eck0eUUb3r66272QS1RbVFBEsB8T4o4vwbdip84Ux+mB KbzA== X-Forwarded-Encrypted: i=1; AJvYcCV55trMOnZj8J7eR8C6/ZENSqV7bc3Co7JwQovvk0EzUtwWGmPldj1YcNKj4nktyd9nAS0=@dpdk.org X-Gm-Message-State: AOJu0YzYwDcGZgNYDrs4DH5P12IG/7QKUxBpc2GyRlkIP9NxEjDVkR08 KJQeCc/DW07ZV6IF/zlV8YHbTEKCP9X2g2PO+45EZ0+9UbEo6ySrcVShFgRJIws= X-Google-Smtp-Source: AGHT+IFtPbBe8G7mzKptTFkr37vGhVSJVvOBkXqS/HVtEVAPl1euLsccBqIXNoYp+sxv1iKRoE1NAw== X-Received: by 2002:a2e:a9a9:0:b0:2f4:9fb:5905 with SMTP id 38308e7fff4ca-2f64d38cf3fmr55373161fa.0.1725528221315; Thu, 05 Sep 2024 02:23:41 -0700 (PDT) Received: from [192.168.200.22] ([84.245.121.62]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-5c3cc699e68sm971187a12.62.2024.09.05.02.23.40 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 05 Sep 2024 02:23:41 -0700 (PDT) Message-ID: Date: Thu, 5 Sep 2024 11:23:38 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 04/12] dts: add mechanism to skip test cases or suites To: Jeremy Spewock Cc: thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, probb@iol.unh.edu, paul.szczepanek@arm.com, Luca.Vizzarro@arm.com, npratte@iol.unh.edu, dmarx@iol.unh.edu, alex.chapman@arm.com, dev@dpdk.org References: <20240301155416.96960-1-juraj.linkes@pantheon.tech> <20240821145315.97974-1-juraj.linkes@pantheon.tech> <20240821145315.97974-5-juraj.linkes@pantheon.tech> Content-Language: en-US From: =?UTF-8?Q?Juraj_Linke=C5=A1?= In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 26. 8. 2024 18:52, Jeremy Spewock wrote: > On Wed, Aug 21, 2024 at 10:53 AM Juraj Linkeš > wrote: > >> --- a/dts/framework/test_result.py >> +++ b/dts/framework/test_result.py >> @@ -75,6 +75,20 @@ def create_config(self) -> TestSuiteConfig: >> test_cases=[test_case.__name__ for test_case in self.test_cases], >> ) >> >> + @property >> + def skip(self) -> bool: >> + """Skip the test suite if all test cases or the suite itself are to be skipped. >> + >> + Returns: >> + :data:`True` if the test suite should be skipped, :data:`False` otherwise. >> + """ >> + all_test_cases_skipped = True >> + for test_case in self.test_cases: >> + if not test_case.skip: >> + all_test_cases_skipped = False >> + break > > You could also potentially implement this using the built-in `all()` > function. It would become a simple one-liner like > `all_test_cases_skipped = all(test_case.skip for test_case in > self.test_cases)`. That's probably short enough to even just put in > the return statement though if you wanted to. > Good catch, I'll use any() here. >> + return all_test_cases_skipped or self.test_suite_class.skip >> + >> >> class Result(Enum): >> """The possible states that a setup, a teardown or a test case may end up in.""" >> @@ -86,12 +100,12 @@ class Result(Enum): >> #: >> ERROR = auto() >> #: >> - SKIP = auto() >> - #: >> BLOCK = auto() >> + #: >> + SKIP = auto() >> >> def __bool__(self) -> bool: >> - """Only PASS is True.""" >> + """Only :attr:`PASS` is True.""" >> return self is self.PASS >> >> >> @@ -169,12 +183,13 @@ def update_setup(self, result: Result, error: Exception | None = None) -> None: >> self.setup_result.result = result >> self.setup_result.error = error >> >> - if result in [Result.BLOCK, Result.ERROR, Result.FAIL]: >> - self.update_teardown(Result.BLOCK) >> - self._block_result() >> + if result != Result.PASS: >> + result_to_mark = Result.BLOCK if result != Result.SKIP else Result.SKIP >> + self.update_teardown(result_to_mark) >> + self._mark_results(result_to_mark) >> >> - def _block_result(self) -> None: >> - r"""Mark the result as :attr:`~Result.BLOCK`\ed. >> + def _mark_results(self, result) -> None: > > Is it worth adding the type annotation for `result` here and to the > other places where this is implemented? I guess it doesn't matter that > much since it is a private method. > I didn't add it precisely because it's a private method and it's pretty self explanatory. >> + """Mark the result as well as the child result as `result`. > > Are these methods even marking their own result or only their > children? It seems like it's only really updating the children > recursively and its result would have already been updated before this > was called. > It's supposed to be just their result which is actually the result of the children in all but the TestCaseResult classes. Conceptually, each results level should contains these: 1. the result of setup 2. the result of teardown 3. the result of the level itself (outside of setup and teardown) The result of the level itself is what's supposed to be set here. The thing is we're making the child results for non-test cases and the result of the test cases for test cases. Maybe I only need to update the docstring. >> >> The blocking of child results should be done in overloaded methods. >> """ > >>