public inbox for ltp@lists.linux.it
 help / color / mirror / Atom feed
From: Joerg Vehlow <lkml@jv-coder.de>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH 1/2] tst_test.sh: Fix calling not yet loaded cleanup function
Date: Tue, 19 May 2020 08:44:56 +0200	[thread overview]
Message-ID: <eb30b7e2-fb61-cdcb-b24b-6deb5512f4a4@jv-coder.de> (raw)
In-Reply-To: <20200519063740.GA7756@dell5510>

Hi,

>> Is it really important, that test is started? Shouldn't it be enough if we
>> got to the point, where the test
>> could be started. Moving TST_RUN_STARTED out of the condition would reduce
>> repetition.
> Well, if you look into the code, there is tst_require_cmds call in if clause,
> which should pass:
>
> 	#TODO check that test reports some results for each test function call
> 	while [ $TST_ITERATIONS -gt 0 ]; do
> 		if [ -n "$TST_TEST_DATA" ]; then
> 			tst_require_cmds cut tr wc
> 			_tst_max=$(( $(echo $TST_TEST_DATA | tr -cd "$TST_TEST_DATA_IFS" | wc -c) +1))
> 			for _tst_i in $(seq $_tst_max); do
> 				_tst_data="$(echo "$TST_TEST_DATA" | cut -d"$TST_TEST_DATA_IFS" -f$_tst_i)"
> 				TST_RUN_STARTED=1
> 				_tst_run_tests "$_tst_data"
> 			done
> 		else
> 			TST_RUN_STARTED=1
> 			_tst_run_tests
> 		fi
> 		TST_ITERATIONS=$((TST_ITERATIONS-1))
> 	done

At this point setup was executed right? So if tst_require_cmds fails, it 
should execute cleanup, to
revert whatever setup did, no?
Oh I guess changing the condition to "setup run _or_ test run" will make 
this work.
But is there really a reason for having both variables? I guess the 
reason they exist at all is to ensure,
that the cleanup function is already defined. But that should be true, 
as soon as tst_run is executed.
So maybe one single variable that is set, when TST_RUN is executed is 
sufficient.
In my opinion the decision to execute of cleanup shouldn't be too 
complicated. The cleanup function
should be able to handle an abort anytime during test execution anyway. 
So it should be callable, as soon
as it is defined. At least when tst_run is executed, it should be ok to 
call cleanup.

J?rg

      reply	other threads:[~2020-05-19  6:44 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-18 13:01 [LTP] [PATCH 1/2] tst_test.sh: Fix calling not yet loaded cleanup function Petr Vorel
2020-05-18 13:01 ` [LTP] [PATCH 2/2] tst_test.sh: Warn about setup/cleanup function not loaded Petr Vorel
2020-05-19  5:10   ` Joerg Vehlow
2020-05-19  6:43     ` Petr Vorel
2020-05-19  5:05 ` [LTP] [PATCH 1/2] tst_test.sh: Fix calling not yet loaded cleanup function Joerg Vehlow
2020-05-19  6:37   ` Petr Vorel
2020-05-19  6:44     ` Joerg Vehlow [this message]

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=eb30b7e2-fb61-cdcb-b24b-6deb5512f4a4@jv-coder.de \
    --to=lkml@jv-coder.de \
    --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