lucc / khard

Console vcard client
https://khard.readthedocs.io/en/latest/
GNU General Public License v3.0
600 stars 65 forks source link

Test failure with Python 3.8 #263

Closed dotlambda closed 4 years ago

dotlambda commented 4 years ago

The testsuite currently results in

======================================================================
FAIL: test_global_help (test.test_command_line_interface.HelpOption)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/nix/store/n0b076p351ma864q38if4yglsg99hw2s-python3-3.8.3/lib/python3.8/unittest/mock.py", line 1325, in patched
    return func(*newargs, **newkeywargs)
  File "/build/khard-0.16.1/test/test_command_line_interface.py", line 48, in test_global_help
    self._test(['-h'], 'usage: TESTSUITE [-h]')
  File "/build/khard-0.16.1/test/test_command_line_interface.py", line 45, in _test
    self.assertTrue(text.startswith(expect))
AssertionError: False is not true

Thus I patched the testsuite to use assertRegex:

@mock.patch('sys.argv', ['TESTSUITE'])
class HelpOption(unittest.TestCase):

    def _test(self, args, expect):
        """Test the command line args and compare the prefix of the output."""
        with self.assertRaises(SystemExit):
            with mock_stream() as stdout:
                cli.parse_args(args)
        text = stdout.getvalue()
        self.assertRegex(text, expect)

    def test_global_help(self):
        self._test(['-h'], r'^usage: TESTSUITE \[-h\]')

and get

======================================================================
FAIL: test_global_help (test.test_command_line_interface.HelpOption)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/nix/store/n0b076p351ma864q38if4yglsg99hw2s-python3-3.8.3/lib/python3.8/unittest/mock.py", line 1325, in patched
    return func(*newargs, **newkeywargs)
  File "/build/khard-0.16.1/test/test_command_line_interface.py", line 48, in test_global_help
    self._test(['-h'], r'^usage: TESTSUITE \[-h\]')
  File "/build/khard-0.16.1/test/test_command_line_interface.py", line 45, in _test
    self.assertRegex(text, expect)
AssertionError: Regex didn't match: '^usage: TESTSUITE \\[-h\\]' not found in 'usage: TESTSUITE\n       [-h]\n       [-c CONFIG]\n       [--debug]\n       [--skip-unparsable]\n       [-v]\n       {list,ls,show,details,template,birthdays,bdays,email,phone,postaddress,post,postaddr,new,add,add-email,merge,edit,modify,ed,copy,cp,move,mv,remove,delete,del,rm,addressbooks,abooks,filename,file,export,source,src}\n       ...\n\npositional arguments:\n  {list,ls,show,details,template,birthdays,bdays,email,phone,postaddress,post,postaddr,new,add,add-email,merge,edit,modify,ed,copy,cp,move,mv,remove,delete,del,rm,addressbooks,abooks,filename,file,export,source,src}\n    list (ls)\n    list all\n    (selected)\n    contacts\n    show (details)\n    display\n    detailed\n    information\n    about one\n    contact\n    template\n    print an\n    empty yaml\n    template\n    birthdays (bdays)\n    list\n    birthdays\n    (sorted by\n    month and\n    day)\n    email\n    list email\n    addresses\n    phone\n    list phone\n    numbers\n    postaddress (post, postaddr)\n    list postal\n    addresses\n    new (add)\n    create a\n    new contact\n    add-email\n    Extract\n    email\n    address\n    from the\n    "From:"\n    field of an\n    email\n    header and\n    add to an\n    existing\n    contact or\n    create a\n    new one\n    merge\n    merge two\n    contacts\n    edit (modify, ed)\n    edit the\n    data of a\n    contact\n    copy (cp)\n    copy a\n    contact to\n    a different\n    addressbook\n    move (mv)\n    move a\n    contact to\n    a different\n    addressbook\n    remove (delete, del, rm)\n    remove a\n    contact\n    addressbooks (abooks)\n    list addres\n    sbooks\n    filename (file)\n    list\n    filenames\n    of all\n    matching\n    contacts\n    export\n    DEPRECATED\n    use \'show -\n    -format=yam\n    l\'\n    source (src)\n    DEPRECATED\n    use \'edit -\n    -format=vca\n    rd\'\n\noptional arguments:\n  -h, --help\n    show this\n    help\n    message and\n    exit\n  -c CONFIG, --config CONFIG\n    config file\n    to use\n  --debug\n    enable\n    debug\n    output\n  --skip-unparsable\n    skip\n    unparsable\n    vcard files\n  -v, --version\n    show\n    program\'s\n    version\n    number and\n    exit\n'

This does not happen when using Python 3.7.

d7415 commented 4 years ago

The original version passes for me with py3.8.3 on master and develop.

lucc commented 4 years ago

I am also wondering as these failures do not show up on travis or my local computer. Can you please report your OS, python version and how you installed it.

dotlambda commented 4 years ago

I'm on NixOS, Python 3.8.3. I build khard from nixpkgs using nix-build -A khard. I tried bisecting nixpkgs to find a culprit but it seems this exists since updating khard from 0.14.0 to 0.15.1.

dotlambda commented 4 years ago

I suspect this might be due to to the sandbox in which Nix builds packages. Maybe the options are printed on multiple lines if the terminal width is too small? The latter might not be set/recognized/... in the sandbox.

lucc commented 4 years ago

I am also sometimes working no nixos. I did not have any luck with any of these:

nix-shell --pure --command 'python setup.py test -A nixos.khard  # error: attribute 'nixos' in selection path 'nixos.khard' not found
nix-shell --pure --command 'python setup.py test -p khard        # missing setuptools

But I hacked together this shell.nix some time ago and it works:

{ pkgs ? import <nixpkgs> {} }:

pkgs.mkShell {
  buildInputs = with pkgs; [
    (python3.withPackages(ps: with ps; [
      atomicwrites
      configobj
      ruamel_yaml
      setuptools
      wheel
      twine
      unidecode
      vobject

      coverage
      sphinx

      importlib-metadata

      pip
    ]))
    hyperfine
    git
    termite # for the terminfo
  ];
  shellHook = ''
            alias pip="PIP_PREFIX='$(pwd)/_build/pip_packages' \pip"
            export PYTHONPATH="$(pwd)/_build/pip_packages/lib/python3.7/site-packages:$PYTHONPATH"
            unset SOURCE_DATE_EPOCH
            export TERM=xterm
  '';
}

I just played around a bit and this seems to be a minimal version that works:

{ pkgs ? import <nixpkgs> {} }:

pkgs.mkShell {
  buildInputs = with pkgs; [
    (python3.withPackages(ps: with ps; [
      setuptools
    ]))
    git
  ];
}

but than setuptools will download the dependencies itself.

I am new to development on nixos (only used arch linux before) and would be happy to learn how to do that "better".

lucc commented 4 years ago

I just checked https://github.com/NixOS/nixpkgs/blob/34b9b139bbf743e88b5a27cc876a979dae1ace85/pkgs/applications/misc/khard/default.nix and it seems that @matthiasbeyer maintains it. Maybe he can help us?

dotlambda commented 4 years ago

I suggest you do nix-shell --pure -A khard in a checkout of the nixpkgs git repo. To get setuptools_scm working you will have to add git to e.g. nativeBuildInputs in pkgs/applications/misc/khard/default.nix. Then you can cd to a directory with the khard source code. Using this, python setup.py test will work fine, suggesting that it's really due to sandboxing,

dotlambda commented 4 years ago

Maybe @FRidh has some insight.

FRidh commented 4 years ago

What does khard list -h show in and outside the sandbox?

dotlambda commented 4 years ago

The output of khard -h inside sandbox:

usage: khard
       [-h]
       [-c CONFIG]
       [--debug]
       [--skip-unparsable]
       [-v]
       {list,ls,show,details,template,birthdays,bdays,email,phone,postaddress,post,postaddr,new,add,add-email,merge,edit,modify,ed,copy,cp,move,mv,remove,delete,del,rm,addressbooks,abooks,filename,file,export,source,src}
       ...

positional arguments:
  {list,ls,show,details,template,birthdays,bdays,email,phone,postaddress,post,postaddr,new,add,add-email,merge,edit,modify,ed,copy,cp,move,mv,remove,delete,del,rm,addressbooks,abooks,filename,file,export,source,src}
    list (ls)
    list all
    (selected)
    contacts
    show (details)
    display
    detailed
    information
    about one
    contact
    template
    print an
    empty yaml
    template
    birthdays (bdays)
    list
    birthdays
    (sorted by
    month and
    day)
    email
    list email
    addresses
    phone
    list phone
    numbers
    postaddress (post, postaddr)
    list postal
    addresses
    new (add)
    create a
    new contact
    add-email
    Extract
    email
    address
    from the
    "From:"
    field of an
    email
    header and
    add to an
    existing
    contact or
    create a
    new one
    merge
    merge two
    contacts
    edit (modify, ed)
    edit the
    data of a
    contact
    copy (cp)
    copy a
    contact to
    a different
    addressbook
    move (mv)
    move a
    contact to
    a different
    addressbook
    remove (delete, del, rm)
    remove a
    contact
    addressbooks (abooks)
    list addres
    sbooks
    filename (file)
    list
    filenames
    of all
    matching
    contacts
    export
    DEPRECATED
    use 'show -
    -format=yam
    l'
    source (src)
    DEPRECATED
    use 'edit -
    -format=vca
    rd'

optional arguments:
  -h, --help
    show this
    help
    message and
    exit
  -c CONFIG, --config CONFIG
    config file
    to use
  --debug
    enable
    debug
    output
  --skip-unparsable
    skip
    unparsable
    vcard files
  -v, --version
    show
    program's
    version
    number and
    exit

outside sandbox:

usage: khard [-h] [-c CONFIG] [--debug] [--skip-unparsable] [-v]
             {list,ls,show,details,template,birthdays,bdays,email,phone,postaddress,post,postaddr,new,add,add-email,merge,edit,modify,ed,copy,cp,move,mv,remove,delete,del,rm,addressbooks,abooks,filename,file,export,source,src}
             ...

positional arguments:
  {list,ls,show,details,template,birthdays,bdays,email,phone,postaddress,post,postaddr,new,add,add-email,merge,edit,modify,ed,copy,cp,move,mv,remove,delete,del,rm,addressbooks,abooks,filename,file,export,source,src}
    list (ls)           list all (selected) contacts
    show (details)      display detailed information about one contact
    template            print an empty yaml template
    birthdays (bdays)   list birthdays (sorted by month and day)
    email               list email addresses
    phone               list phone numbers
    postaddress (post, postaddr)
                        list postal addresses
    new (add)           create a new contact
    add-email           Extract email address from the "From:" field of an email header and add to an
                        existing contact or create a new one
    merge               merge two contacts
    edit (modify, ed)   edit the data of a contact
    copy (cp)           copy a contact to a different addressbook
    move (mv)           move a contact to a different addressbook
    remove (delete, del, rm)
                        remove a contact
    addressbooks (abooks)
                        list addressbooks
    filename (file)     list filenames of all matching contacts
    export              DEPRECATED use 'show --format=yaml'
    source (src)        DEPRECATED use 'edit --format=vcard'

optional arguments:
  -h, --help            show this help message and exit
  -c CONFIG, --config CONFIG
                        config file to use
  --debug               enable debug output
  --skip-unparsable     skip unparsable vcard files
  -v, --version         show program's version number and exit

I got these by setting doCheck = false and adding $out/bin/khard -h to postInstall.

lucc commented 4 years ago

@dotlambda I conclude that the sandbox is somehow breaking things here. I am not sure if I can help with that but you can surely use this issue to discuss it.

dotlambda commented 4 years ago

This is the commit that changed the argparse behaviour for Python 3.8: https://github.com/python/cpython/commit/74102c9a5f2327c4fc47feefa072854a53551d1f Here's where the value for get_terminal_size().columns is set: https://github.com/python/cpython/blob/3.8/Lib/shutil.py#L1318 Thus the easiest fix for nixpkgs would be export COLUMNS=80.

FRidh commented 4 years ago

@dotlambda please check khard list -h, that's what is used in that test I think.

FRidh commented 4 years ago

Oh that's a surprising cause

dotlambda commented 4 years ago

@dotlambda please check khard list -h, that's what is used in that test I think.

That doesn't work because OSError: Config file not found: "/home/rschuetz/.config/khard/khard.conf". However the test actually tests khard -h, khard list -h, and khard -h list. I think the outcome would be the same for the others.