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
PICARD-1063: Fix astrcmp tests if not compiled #696
PICARD-1063: Fix astrcmp tests if not compiled #696
Conversation
test/test_util_astrcmp.py
Outdated
class AstrcmpCTest(AstrcmpBase, unittest.TestCase): | ||
func = astrcmp_c | ||
|
||
from picard.util.astrcmp import astrcmp_py |
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.
Why was this shifted from the top of the module?
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.
Because it goes with the test immediately below and compares and contrasts with the one for the C module.
test/test_util_astrcmp.py
Outdated
class AstrcmpCTest(AstrcmpBase, unittest.TestCase): | ||
func = astrcmp_c | ||
except: | ||
pass |
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.
Add an endline to the file. Please rebase the above changes.
3aedafa
to
9c93354
Compare
I hope that this is what you wanted. |
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.
I think something like the following is a better approach:
diff --git a/test/test_util_astrcmp.py b/test/test_util_astrcmp.py
index 5198f5ac..fce8c779 100644
--- a/test/test_util_astrcmp.py
+++ b/test/test_util_astrcmp.py
@@ -3,7 +3,12 @@
import os
import os.path
import unittest
-from picard.util.astrcmp import astrcmp_c, astrcmp_py
+from picard.util.astrcmp import astrcmp_py
+
+try:
+ from picard.util.astrcmp import astrcmp_c
+except ImportError:
+ astrcmp_c = None
class AstrcmpBase(object):
@@ -23,6 +28,9 @@ class AstrcmpBase(object):
class AstrcmpCTest(AstrcmpBase, unittest.TestCase):
func = astrcmp_c
+ @unittest.skipIf(astrcmp_c is None, "hallo")
+ def test_astrcmp(self):
+ super()
class AstrcmpPyTest(AstrcmpBase, unittest.TestCase):
func = astrcmp_py
This way, you'll get proper skipped indicators and the number of test executed is always constant.
Agreed - I will make this change. |
@mineo Definitely a better solution than mine - fix pushed. |
Thanks to @mineo for his help.
After #689 tests fail if astrcmp is not compiled.
Resolves https://tickets.metabrainz.org/browse/PICARD-1063