API Tests: rename test & delete test #321
Conversation
|
Merging to
|
tests/api/test_api.py
Outdated
| tests that when the cript.API is given None for host, api_token, storage_token that it can correctly | ||
| retrieve things from the env variable | ||
| tests that when the cript.API is given `None` for `host`, `api_token`, `storage_token` | ||
| that it can correctly retrieve them from the environment variables | ||
| """ | ||
| host_value = "http://development.api.mycriptapp.org/" |
There was a problem hiding this comment.
Shouldn't we have those exported in the env anyways?
When you run the tests?
It can be confusing, if you set your env variables to connect to staging and then you run the tests and they fail because development is down.
There was a problem hiding this comment.
yes, but I thought this would be an easier way to write the tests so its modular and not dependent on the CI variables, so we can change the CI variables without this going off that it needs to be changed too. It is more self contained in the function, easier to read, and easier to write assertions for.
I can change it if we think getting it directly from the CI is better, it is not a huge deal to me.
There was a problem hiding this comment.
I think we as knowledgable engineers that run the tests should be comfortable with ENV variables.
I'd like it consistent with them
There was a problem hiding this comment.
I think we as knowledgable engineers
I appreciate the vote of confidence, but let me tell you, I've made some of the dumbest mistakes before 😂
but yeah I can change it to depend on the environment instead of being in the function
nh916
changed the title
nh916
changed the title
Description
uncommented test and changed test name to be self documenting
Changes
test_create_apito allow it to be usable againtest_api_cript_env_varstotest_create_api_with_noneTests
Known Issues
Notes
Checklist
CONTRIBUTORS.md) in the pull request source branch.