Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a test suite and a first test #96

Merged
merged 8 commits into from
Jan 13, 2018
Merged

Add a test suite and a first test #96

merged 8 commits into from
Jan 13, 2018

Conversation

lucc
Copy link
Owner

@lucc lucc commented Sep 21, 2016

If the infrastructure is in place we can start to add tests for different things.

The first things that come to mind (and that might already be somewhat complicatet) are

  • parsing config files
  • parsing example vcard files
  • exporting and reimporting contacts

If we write tests that interact with the vcard store and the config file we have to take care to never use the config file and vcards of the user.

Fixes #62.

@lucc
Copy link
Owner Author

lucc commented Sep 25, 2016

Should I add more tests here or wait for this to be merged and open new PRs for new tests?

@scheibler
Copy link
Collaborator

Definitely necessary. Thanks.

The command line test case looks like a good start. But I've a problem with the
test_khard_runner_script method. My khard python environment is located within a virtual environment
while the khard-runner.py interpreter points to the default python location. Therefore the
corresponding test fails.

The only workaround I found so far is to change the khard-runner.py interpreter to my custom python
installation. But I would have to undo that change before every commit. Can you eventually think of
a better solution for that?

The test can be run with `python setup.py test` but no actual test cases where
added yet.
This will make it possible to pass different arguments in tests without
starting new processes.
This makes it possible to run the tests with `python setup.py test` and with
`python -m unittest`.
The unittest.mock module can replace all custom mocking of resources that was
previously done manually.
Executing another process is slow and does not work in a virtualenv as the
wrong interpreter is given in that file.
@lucc
Copy link
Owner Author

lucc commented Feb 23, 2017

I learned some new tricks when we started to write unittests for alot so I updated the mocking part of the tests.

I removed the khard-runner test for now. We can resurrect it from the git history if we want. I will try to spend as little time as possible on testing the cli as I think it is the least important part to test (also something I learned from the alot tests).

Would you be interested in running the tests on travis?

@strk
Copy link

strk commented May 16, 2017

May be a good idea to also add a .travis.yml in this PR, and enable it for your fork , @lucc ?

@lucc
Copy link
Owner Author

lucc commented Aug 4, 2017

@strk it is here.

@strk
Copy link

strk commented Aug 6, 2017

Great work !

@josephholsten
Copy link

@scheibler what's remaining to merge this?

@andrey-utkin
Copy link

Gentoo package app-misc/khard maintainers appreciate this work on actual runnable tests. Waiting for resolution.

andrey-utkin pushed a commit to andrey-utkin/gentoo that referenced this pull request Dec 17, 2017
khard is a console carddav client, from same author as app-misc/khal.

Adding it to main gentoo repo fulfills bug #601440.
Previously khard was available from xmw overlay, which is no longer
available.

Based on original ebuild by Kristian <kbx@posteo.de>.

Test phase will be implemented when upstream gets a test suite, waiting
for resolution of lucc/khard#96 .

Suggested-by: Kristian <kbx@posteo.de>
Reviewed-by: Andrey Utkin <andrey_utkin@gentoo.org>
Link: https://bugs.gentoo.org/601440
Package-Manager: Portage-2.3.17, Repoman-2.3.6
gentoo-bot pushed a commit to gentoo/gentoo that referenced this pull request Dec 17, 2017
khard is a console CardDAV client, from same author as app-misc/khal.

Adding it to main gentoo repo fulfills bug #601440.
Previously khard was in xmw overlay, which is no longer available.

Based on original ebuild by Kristian <kbx@posteo.de>.

Test phase will be implemented when upstream gets a test suite, waiting
for resolution of lucc/khard#96 .

Bug: https://bugs.gentoo.org/601440
Suggested-by: Kristian <kbx@posteo.de>
Reviewed-by: Andrey Utkin <andrey_utkin@gentoo.org>
Tested-by: Kristian <kbx@posteo.de>
Package-Manager: Portage-2.3.17, Repoman-2.3.6
@lucc
Copy link
Owner Author

lucc commented Jan 12, 2018

@scheibler I would like to merge this rather sooner than later and also would ask you to go to travis and activate tests for this repository. You could configure it to run tests on every PR.

@lucc
Copy link
Owner Author

lucc commented Jan 12, 2018

By the way these test also pass when I merge them with current master locally :)

@josephholsten
Copy link

I’ve requested pimutils give us a place to maintain a fork of khard: pimutils/meta#10

This is pretty near the top of list of things I hope we can resolve once we have a new home!

@scheibler
Copy link
Collaborator

@lucc: travis should be activated for the khard repo. Feel free to merge and test.

@josephholsten: Please see my update on #144

@lucc lucc merged commit d2710f5 into lucc:develop Jan 13, 2018
@lucc lucc deleted the tests branch January 13, 2018 08:37
@lucc
Copy link
Owner Author

lucc commented Jan 13, 2018

Thank you @scheibler !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants