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

[Tests] refactor tests #167

Merged
merged 4 commits into from
May 19, 2020
Merged

[Tests] refactor tests #167

merged 4 commits into from
May 19, 2020

Conversation

patrickvonplaten
Copy link
Contributor

@patrickvonplaten patrickvonplaten commented May 19, 2020

This PR separates AWS and Local tests to remove these ugly statements in the script:

        if "/" not in dataset_name:
            logging.info("Skip {} because it is a canonical dataset")
            return

To run a aws test, one should now run the following command:

pytest -s tests/test_dataset_common.py::AWSDatasetTest::test_builder_class_wmt14

The same local test, can be run with:

pytest -s tests/test_dataset_common.py::LocalDatasetTest::test_builder_class_wmt14

@patrickvonplaten patrickvonplaten changed the title refactor tests [Tests] refactor tests May 19, 2020
@lhoestq
Copy link
Member

lhoestq commented May 19, 2020

Nice !

from __future__ import absolute_import
from __future__ import division
from __future__ import print_function
from __future__ import absolute_import, division, print_function
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a problem with black smh ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know whether that's black or isort, but I don't think it's a problem.

It's just that we don't check quality on the datasets folder at the moment - see the .circle_ci folder, but I added ./datasets to the Make style file. So I think it's just that some people upload datasets without formatting them and I correct it :D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, we should add ./datasets to the circle ci quality check!

@patrickvonplaten patrickvonplaten merged commit 4263a4f into master May 19, 2020
@patrickvonplaten patrickvonplaten deleted the refactor_tests branch May 19, 2020 16:17
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.

2 participants