Rahix / tbot

Automation/Testing tool for Embedded Linux Development
https://tbot.tools
GNU General Public License v3.0
84 stars 21 forks source link

shell.copy() does not accept globs in source path #75

Closed yanivy-nr closed 1 year ago

yanivy-nr commented 1 year ago

Hello,

When trying to copy multiple files filtered via glob to the device, shell.copy() fails with the following error:

tests/some_test.py:183: in copy_file
    shell.copy(*sp.glob('*'), dp)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

args = (Path(<tbot.selectable.LocalLabHost object at 0x7f5de90249f0>, '/home/ubuntu/tbot/tests/data/file1'), Path(<tbot.selec....py'), Path(<tbot.selectable.LocalLabHost object at 0x7f5de90249f0>, '/home/ubuntu/tbot/tests/data/file2'), ...)
kwargs = {}

    @functools.wraps(tc)
    def wrapped(*args: typing.Any, **kwargs: typing.Any) -> typing.Any:
        with tbot.testcase(tc.__name__):
>           return tc(*args, **kwargs)
E           TypeError: copy() takes 2 positional arguments but 7 were given

venv/lib/python3.8/site-packages/tbot/decorators.py:62: TypeError

Below is the code we use for copying files:

    with tbot.ctx.request(tbot.role.LabHost) as lh, \
        tbot.ctx.request(tbot.role.BoardLinux) as lnx:

        sp = linux.Path(lh, '/home/ubuntu/tbot/tests/data')
        dp = linux.Path(lnx, '/home/root')
        shell.copy(*sp.glob('*'), dp)

The current workaround is to get the file list from the glob and using a loop to copy the files:

    with tbot.ctx.request(tbot.role.LabHost) as lh, \
        tbot.ctx.request(tbot.role.BoardLinux) as lnx:

        sp = linux.Path(lh, '/home/ubuntu/tbot/tests/data')
        dp = linux.Path(lnx, '/home/root')
        files = sp.glob('*')
        for f in files:
             shell.copy(f, dp)

It would be great to have shell.copy() support copying files with glob.

thanks, Yaniv

Rahix commented 1 year ago

Hi, thank you very much for raising this topic.

I don't want to make copy() able to do this because it is already quite a complex function. Instead, I prefer building a helper ontop of copy() which can do exactly what you are asking for.

I've done this (and quite a few other related changes) in PR #77. The important commit is f8069544d1e5 which adds the function in question. I've included some examples of the intended usage in its documentation.

Maybe you can checkout that branch and give me some feedback whether this works for your usecase?

Rahix commented 1 year ago

I merged the changes now anyway, but I'd still be happy to hear if it helps you with your use-case. If it doesn't, we can reopen this issue.