Thomas Haller <thaller@xxxxxxxxxx> wrote:
> > +check_features()
> > +{
> > + for ffilename in features/*.nft; do
> > + feature=${ffilename#*/}
> > + feature=${feature%*.nft}
> > + eval NFT_HAVE_${feature}=0
> > + $NFT -f "$ffilename" 2>/dev/null
>
> is the "--check" option here missing? At least, the commit message says
Yes, added.
> I think it should run in a separate netns too.
Should not make a difference due to --check, nothing
is altered.
> OK, that's nice, to see in the output.
>
> But why this "nft -f" specific detection? Why not just executable
> scripts?
Because I want it to be simple, it will be enough to drop a ruleset
with the feature into features/ durectory and run-tests.sh would do the
reset.
While its possible to launch nft as a script via
#!/usr/bin/env nft
This would use the system nft, not the newly built one. And fudging
with $PATH seems ugly...
> Also, why should "run-tests.sh" pre-evaluate those NFT_HAVE_* features?
> Just let each tests:
>
> if ! "$BASEDIR/features/chain-binding" ; then
> echo " defaultchain"
> return
> fi
>
> then the checks are more flexible (arbitrary executables).
I could do that, but I don't see the need for arbitrary scripts so far.
> Downside, if the check is time consuming (which it shouldn't),
Agree, it should not.
> then
> tests might call it over and over. Workaround for that: have "run-
> tests.sh" prepare a temporary directory and export it as
> $NFT_TEST_TMPDIR". Then "features/chain-binding" can cache the result
> there. "run-tests.sh" would delete the directory afterwards.
>
> If you want to print those features, you can still let run-tests.sh
> iterate over "$BASEDIR"/features/* and print the result.
Yes, thats a working alternative but I don't see the need for this.
> > diff --git a/tests/shell/testcases/transactions/30s-stress
> > b/tests/shell/testcases/transactions/30s-stress
> > index 4d3317e22b0c..924e7e28f97e 100755
> > --- a/tests/shell/testcases/transactions/30s-stress
> > +++ b/tests/shell/testcases/transactions/30s-stress
> > @@ -16,6 +16,18 @@ if [ x = x"$NFT" ] ; then
> > NFT=nft
> > fi
> >
> > +if [ -z $NFT_HAVE_chain_binding ];then
> > + NFT_HAVE_chain_binding=0
> > + mydir=$(dirname $0)
>
> I think run-tests.sh should export the base directory, like "$BASEDIR",
> or "$NFT_TEST_BASEDIR". Tests should use it (and rely to have it).
Can do that.
> ah, you'd have each tests re-implement the check? So that they can run
> without the "run-tests.sh" wrapper?
No, just 30s-stress. This test is special because its random by nature
and it makes sense to run it for multiple hours once in a while.
> The point here is, that if the "run-tests.sh" wrapper can provide
> something useful, then tests should be able to rely on it (and don't
> implement a fallback path).
Yes, no other script will do this.
I'll send an updated batch soon to see how this looks like with more
feature tests in place.
