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

How to run tests with Pipenv #161

Merged
merged 2 commits into from Oct 29, 2018
Merged

How to run tests with Pipenv #161

merged 2 commits into from Oct 29, 2018

Conversation

hjacobs
Copy link
Contributor

@hjacobs hjacobs commented Oct 18, 2018

Goal: make it easier for contributors to get started.

  • document how to run tests in README (only prerequisite: Pipenv)
  • make py.test work from the top-level folder (do not depend on relative path, but use __file__)
  • change some string paths to use pathlib.Path instead (part of stdlib since Python 3.4)

Fixes #159.

@hjacobs hjacobs changed the title WIP: how to run tests with Pipenv How to run tests with Pipenv Oct 18, 2018
@stefan-it
Copy link
Member

👍 for introducing pathlib :)

I think it would also be a good idea to replace the .format() constructs with f-strings? They're available with Python 3.6 (and >= 3.6 is recommended by flair) :)

@hjacobs
Copy link
Contributor Author

hjacobs commented Oct 18, 2018

@stefan-it yes, I totally agree and also told @alanakbik about both pathlib and f-strings. I just did not want to change too many things at once (even pathlib is only introduced for now where needed to make the change work).

@hjacobs
Copy link
Contributor Author

hjacobs commented Oct 18, 2018

I also propose getting rid of the TRAVIS env var to skip tests and instead have some flag to run all tests ("slow tests" or similar). The default unit tests should be self-contained and fast.

@tabergma
Copy link
Collaborator

👍
Thanks for the PR! I created two issues out of your suggestions:
#177
#176
Feel free to contribute to them :)

@tabergma tabergma merged commit d311c5d into flairNLP:master Oct 29, 2018
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