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

2.0 #474

Open
wants to merge 24 commits into
base: 2.0
Choose a base branch
from
Open

2.0 #474

wants to merge 24 commits into from

Conversation

sfermigier
Copy link
Contributor

Fix tests, add tox config, modernise code (py 3.6 only).

@KhaledTo
Copy link
Contributor

KhaledTo commented Jul 8, 2019

@sfermigier, can you explain how does it fix tests? To me it looks like you are commenting Test Suites, or is there something I'm missing? Anyways it's cool to see someone is still contributing to this project.

@sfermigier
Copy link
Contributor Author

In commit 6af2aa9 I'm getting rid of the TestSuite objects which are redundant when using, as we do, pytest (pytest does its own discovery of the tests it has to run).

This fixes the two errors that were reported before this commit, due to non-existing test cases (ModelTestCase and AggregationBrowserTestCase) being put in the test suites without being defined.


    def test_suite():
        suite = unittest.TestSuite()

        suite.addTest(unittest.makeSuite(AttributeTestCase))
        suite.addTest(unittest.makeSuite(LevelTestCase))
        suite.addTest(unittest.makeSuite(HierarchyTestCase))
        suite.addTest(unittest.makeSuite(DimensionTestCase))
        suite.addTest(unittest.makeSuite(CubeTestCase))
>       suite.addTest(unittest.makeSuite(ModelTestCase))
E       NameError: name 'ModelTestCase' is not defined

tests/metadata/test_model.py:937: NameError
____________________________________________________________________________________ test_suite _____________________________________________________________________________________

    def test_suite():
        suite = unittest.TestSuite()

>       suite.addTest(unittest.makeSuite(AggregationBrowserTestCase))
E       NameError: name 'AggregationBrowserTestCase' is not defined

Also:

before this commit:

2 failed, 108 passed, 58 skipped, 4 warnings in 1.81 seconds

After this commit:

108 passed, 58 skipped, 10 warnings in 3.30 seconds

in other words, we have 110 non-skipped tests in both cases.

@KhaledTo
Copy link
Contributor

KhaledTo commented Jul 9, 2019

Ok makes sense, also I forgot you branched from 2.0. Thank you @sfermigier.

- Add type hints generated by MonkeyType (some tweaks are still needed).
- Fix some flake8 warnings (including a few real issues)
- Remove some useless imports / sort imports
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

2 participants