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

Change: switch from flask_script to click, add CLI unit tests and upgrade Flask version #1458

Merged
merged 1 commit into from
Dec 6, 2016

Conversation

washort
Copy link

@washort washort commented Dec 6, 2016

This provides basic test coverage for the CLI commands and replaces flask_script.

Closes #1112.

@@ -2,62 +2,75 @@
"""
CLI to manage redash.
"""
import code
Copy link
Member

Choose a reason for hiding this comment

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

Seems unused?

Copy link
Author

Choose a reason for hiding this comment

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

Oops, that's when I was trying to add a shell command and not noticing flask already provided one.

self.assertEqual(result.exit_code, 1)
self.assertIn('not found', result.output)

## XXX figure out how to capture celery tasks kicked off in a subprocess???
Copy link
Member

Choose a reason for hiding this comment

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

I usually mock the method that submits the task to verify it was called.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, the problem is that the click testing stuff runs the code in a subprocess, so you can't directly mock. I think I might split the actual work into a separate function and test it separately.

Copy link
Author

Choose a reason for hiding this comment

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

I was wrong, it just captures stdio. Test fixed.

@arikfr arikfr changed the title Switch from flask_script to click, add CLI unit tests. Change: switch from flask_script to click, add CLI unit tests and upgrade Flask version Dec 6, 2016
@arikfr arikfr merged commit dde55e7 into getredash:master Dec 6, 2016
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