[U-Boot] [PATCH v2 2/2] test/py: add test for whitelist of variables while importing environment

Quentin Schulz quentin.schulz at bootlin.com
Mon May 21 07:43:06 UTC 2018


Hi Stephen,

On Fri, May 18, 2018 at 10:04:05AM -0600, Stephen Warren wrote:
> On 05/18/2018 08:45 AM, Quentin Schulz wrote:
> > This tests that the importing of an environment with a specified
> > whitelist works as intended.
> > 
> > If the variable whitelisted_vars is not set, the env importing should
> > fail with a given message.
> > 
> > For each variable separated by spaces in whitelisted_vars, if
> >   - foo is bar in current env and bar2 in exported env, after importing
> >   exported env, foo shall be bar2,
> >   - foo does not exist in current env and foo is bar2 in exported env,
> >   after importing exported env, foo shall be bar2,
> >   - foo is bar in current env and does not exist in exported env (but is
> >   in the whitelisted_vars), after importing exported env, foo shall be
> >   empty,
> > 
> > Any variable not in whitelisted_vars should be left untouched.
> 
> Acked-by: Stephen Warren <swarren at nvidia.com>
> 
> > diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py
> 
> > +def test_env_import_whitelist(state_test_env):
> 
> > +    state_test_env.u_boot_console.run_command('env import -w %s' % addr)
> > +
> > +    validate_set(state_test_env, "foo1", "bar1")
> > +    validate_set(state_test_env, "foo2", "bar2")
> > +    validate_set(state_test_env, "foo3", "bar3")
> > +    validate_empty(state_test_env, "foo4")
> 
> It might make sense to iterate over *all* variables and make sure that they
> have the same value before/after the import. But, the current code seems to
> cover the basic cases, so it's probably not strictly necessary to do that.

I would advocate for as many test sets as possible, each one testing a
very specific scenario. That way, it's easy to know which scenario
failed instead of having to find within a big scenario what failed and
why. Here, I just want to test that importing some env variables work.

It would make sense to have a small test that tests that setting an
environment variable works (that what's done with test_env_set* test
functions I think) and others to test the exporting of variables and the
importing of some others with all the possible/impossible arguments of
these cli commands. I wish I could test this importing without manually
exporting an environnement before to reduce "dependencies" of the test
and thus keep the test scenario to the actual command we're testing.

However the more the arguments of the commands, the more functions to
write ("exponentially") as we have to test each and every case. The best
would be to also have tests for expected and wanted failures. I think
it's a good start for now but more needs to be done in the env testing
regarding (at least) env exporting and importing.

Quentin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180521/92f4b525/attachment.sig>


More information about the U-Boot mailing list