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

[WIP] 91% test coverage of onionshare/common.py #415

Closed
wants to merge 58 commits into from
Closed

[WIP] 91% test coverage of onionshare/common.py #415

wants to merge 58 commits into from

Conversation

delirious-lettuce
Copy link
Contributor

@delirious-lettuce delirious-lettuce commented May 25, 2017

@micahflee ,

This PR is still a work-in-progress but I wanted to give you an idea of what I've been doing to help with #404 . I will continue to work on it to try and achieve 100% coverage.

Let me know what you think!

@delirious-lettuce delirious-lettuce changed the title [WIP] 72% test coverage of onionshare/common.py [WIP] 91% test coverage of onionshare/common.py May 27, 2017
@delirious-lettuce
Copy link
Contributor Author

delirious-lettuce commented May 30, 2017

@micahflee ,

Yes, it's still in progress. I have some more work to do but I will try to kick it into high gear and get them done quickly.

I will make the changes you asked for in reference to nose to pytest.

Some of the pytest issues may be because the features I used are from a newer version of pytest than what is in the ubuntu/fedora repos. I'll try and figure out how to account for this (whether using a newer pytest from pip or backporting these tests to run with the older version of pytest). I'm not anywhere near as advanced as you so I would defer to you on whether using system installed versions are better than pip installed versions.

I saw this link as I've been going back and forth through the pytest docs and I thought I should be using fixtures instead of setup/teardown methods. But if that's what's causing the issues for you, I will convert them back to try and fix that issue of using an older version of pytest from the OS repos.

I was having similar problems on Travis because it didn't allow access to system installed Flask so I temporarily replaced it with pip install Flask and that allowed it the import to work (and the tests to subsequently run).

Thanks for your time, I will get working on these issues and will let you know when I have them solved!

micahflee and others added 18 commits May 30, 2017 12:21
…`format_seconds`, add initial ZipWriter tests
* add new imports
* rename ZipWriter regex
* add more fixtures
* use classes to group related tests
* adjust expected results after # ? merge
* parametrize `get_available_port` test
* test three platforms instead of one
* initial tests for `get_resource_path` (work-in-progress)
* pass fixtures as args instead of `usefixtures` decorators
* add more tests for `ZipWriter.add_file
* ensure temporary files and directories are removed after tests
…common_test

# Conflicts:
#	test/onionshare_common_test.py
@delirious-lettuce
Copy link
Contributor Author

I messed this up so I'm going to close this PR and re-submit it. Sorry for the confusion!

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

Successfully merging this pull request may close these issues.

None yet

3 participants