Code review comment for lp://staging/~liuyq0307/lava-android-test/exclude-shells-commands-hostshells-test-from-list

Revision history for this message
Andy Doan (doanac) wrote :

This is change concerns me a bit:

> def list_test(self):
> - return self.list_mod(test_definitions.__path__)
> + providers = self.get_test_provider_list()
> + avoid_dirs = []
> + for provider in providers:
> + test_prefix = provider.test_prefix
> + if test_prefix:
> + avoid_dirs.append('%ss' % test_prefix)
> + test_list = self.list_mod(test_definitions.__path__)
> + if not avoid_dirs:
> + return test_list
> + else:
> + ret_list = []
> + for test_id in test_list:
> + if not test_id in avoid_dirs:
> + ret_list.append(test_id)
> + return ret_list
>

The way avoid_dirs gets built is based on your current naming
convention. Instead of guessing avoid dirs that way, why not do
something like:

  def _get_dirs(pathname):
    return [x for x in os.listdir(pathname) if os.path.isdir(x)]

then you could just do:
   avoid_dirs = _get_dirs(test_definitions.__path__)

I'm not a big fan of the logic for removing those items from the list.
The "if not avoid_dirs:" check isn't needed.

« Back to merge proposal