-
-
Notifications
You must be signed in to change notification settings - Fork 224
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
Convert existing tests to pytest + increase coverage #1255
Convert existing tests to pytest + increase coverage #1255
Conversation
Disable failing test Removed unused import
…ture/convert_existing_tests_to_pytest
@marcelotduarte - Thanks for reviewing / deciding on outcomes to the other PR's - with those done i have now updated this PR. Please feel free to try executing the tests and generating the coverage report and let me know what you think. - If you like this direction Cheers and have a good day/night! :) |
Hi! In two or three days I'll try to make time to test and review. Good afternoon! ;-) |
No stress, Just wanted to square this one up so i could start on the more complex modules. I may start suggesting small refactors on those other PR's to make code more "testable" :) |
…ing_tests_to_pytest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.coverage should be ignored at .gitignore?
|
||
@pytest.mark.skip("Test skipped, uncertain if no longer supported - waiting for maintainer to comment on:" | ||
"https://github.com/marcelotduarte/cx_Freeze/pull/1234") | ||
def test_FindModule_from_zip(self, fix_module_finder, fix_samples_dir, tmpdir): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test can be marked as skipped for now. In the rewrite of ModuleFinder, I ignored this method, but similar support for importing directing from the wheel (.whl)... Maybe we can delete it. I'll think about it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your shout - the current usage of @pytest.mark.skip
disables execution of the test - if you want to remove the underlying code + test go for it. I didn't want to remove existing tests without discussion with you :)
In windows:
In Linux:
|
There's 2 coverage files, one is the
Makes sense! - Prob will fail on OSX as well 🤔 ... contemplating how to handle this for testing - will get back to you.... |
I'm talking about a binary (db) file that appears as first file at https://github.com/marcelotduarte/cx_Freeze/pull/1255/files |
Oh bugger! well spotted :) will fix ( am fixing failing test right now - gimmi 10min :) ) |
@marcelotduarte - Test should be fixed now + excess file nuked + gitignore updated :) |
|
Bah! Ok - will fix that up later - watch this space :) |
@marcelotduarte - sorry for delay, have changed approach slightly lets see if it passes now ( Will happily refine tests later, but right now would like to get framework switched over so i can move onto the other tests :) ) |
|
Bah! Ok - Disabled the test for now to unblock the PR. I'll address this in a future PR :) |
I think that you can use skipif in the non-windows platform. |
yep i'll split the test into platform specific tests for the platform specific modules - for now ive just disabled the whole thing as its a "New" test and i don't want it to get in the way if you approve of the move to pytest :) |
I merged so you can adjust. If you can make small patches to facilitate it will be good, but when this is not possible, for now, in this part of adjusting the tests, no problem. |
Problem - Unit test coverage of the
cx_freeze
project is very low - prior to Pull-1234 the majority were also not working.Improving test coverage of the product code will support development of
cx_freeze
by enabling developers and contributors to have a degree of confidence that there changes are non-destructive to existing functionality and expected usage.Proposed Solution - This PR replaces the existing
nose
tests with apytest
suite preconfigured to generate coverage reports as required ( Article here going over benefits of PyTest ) as well as these new tests:test__init__.py
->test_exposed_namespaces
test_exception
->test_raise_exceptions
test_winmsvcr.py
->test_FILES
&test_FILES_TO_DUPLICATE
test_winversioninfo.py
->test___init__
andtest___init__pads_short_versions
A
readme.md
is provided in the/tests
folder explaining how to execute the tests.With this PR total coverage of the suite is raised to
27%
... While this is in review - i will begin writing coverage for more modules ( slowly building up to addressing : Issue-594