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

TST: Add a fixture enabling SRTM login using envvars. #1145

Merged
merged 2 commits into from
Nov 12, 2018

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Oct 12, 2018

Rationale

There is currently no way to run SRTM tests, so they tend to fall into disrepair (cf #1080).

Implications

If you set SRTM_USERNAME and SRTM_PASSWORD in the environment, you can run pytest and have the SRTM tests work. In the future, we may want to define an encrypted variable on Travis to enable running these on CI. I don't want to do this myself as I'd rather someone from the Met Office handle the credentials.

If you set SRTM_USERNAME and SRTM_PASSWORD in the environment, you can
run pytest and have the SRTM tests work.
@QuLogic
Copy link
Member Author

QuLogic commented Oct 12, 2018

Note, you will need to try this out locally since we don't have the envvars set on any CI.

Either we have no credentials and the tests will be skipped, or we do
have the credentials and they will pass, so there's no need to show the
warning.

This needed moving the warning around a bit so it's not triggered just
by importing the srtm package.
@dopplershift dopplershift added this to the 0.17 milestone Nov 1, 2018
Copy link
Contributor

@dopplershift dopplershift left a comment

Choose a reason for hiding this comment

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

Bad tests hitting external stuff, but this at least makes them usable.

@dopplershift dopplershift merged commit b136155 into SciTools:master Nov 12, 2018
@QuLogic QuLogic deleted the srtm-test-login branch November 12, 2018 22:03
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