Conversation
codie3611
left a comment
codie3611
left a comment
There was a problem hiding this comment.
Mostly good, glad to see your first shot turned out so well. Please mock away the side-effects completely. Then it generally works for me. This is the library function from geopy and of course the environment variable reader. Keep it going man! 🚀 🚀 🚀
| # Testing | ||
| - name: Test code | ||
| env: | ||
| TNT_EX1_OPENWEATHERMAP_API_KEY: ${{ secrets.TNT_EX1_OPENWEATHERMAP_API_KEY}} |
There was a problem hiding this comment.
This is not supposed to happen 🤯 you ought to mock the environment variable away. Unit test means to test a standalone unit 😘
| env: | ||
| TNT_EX1_OPENWEATHERMAP_API_KEY: ${{ secrets.TNT_EX1_OPENWEATHERMAP_API_KEY}} | ||
| run: | ||
| ./bin/task test No newline at end of file |
There was a problem hiding this comment.
Please add a newline at the end of the file since some tools don't execute the last line if it ends with EOF (end of file) instead of a newline \n.
| poetry run pytest | ||
| -s | ||
| --cov=. | ||
| --cov-report=html No newline at end of file |
| cmds: | ||
| - > | ||
| poetry run pytest | ||
| -s |
There was a problem hiding this comment.
Pleas write out command line options if you can. I have no fricking clue what -s is ❓
| coords_berlin = fetch_location_coords("Berlin") | ||
| coords_difference = [sum(x) for x in zip(coords_berlin, (-52.520008, -13.404954))] | ||
| assert sum(list(coords_difference)) < 0.01 |
There was a problem hiding this comment.
Interesting way to test this 😅 is there maybe an easier and more transparent way to possible test this? Like checking if something equals something without summing up and other tricks?
| timeout=15, | ||
| ) | ||
| data = response.json() | ||
| data = fetch_new_data(api_key, location_coords) |
There was a problem hiding this comment.
This is so beautiful and smart 💡 I'm proud of you 😘
|
|
||
| @mock.patch("weather_data_fetcher.fetch_new_data", autospec=True) | ||
| def test_fetch_data(self, request_mock): | ||
| request_mock.return_value = { |
There was a problem hiding this comment.
You can make your life easier here and just put into the json payload what you extract in the end. This makes everything much much shorter.
| if "TNT_EX1_OPENWEATHERMAP_API_KEY" not in os.environ: | ||
| dotenv_file = dotenv.find_dotenv() | ||
| dotenv.load_dotenv(dotenv_file) |
There was a problem hiding this comment.
Please remove this and mock the environment variable getter.
| dotenv_file = dotenv.find_dotenv() | ||
| dotenv.load_dotenv(dotenv_file) | ||
|
|
||
| fetch_weather_data(os.environ["TNT_EX1_OPENWEATHERMAP_API_KEY"], (52.520008, 13.404954)) |
There was a problem hiding this comment.
It is nice how you call the function here, but you only check that it does not error out basically. If you really want to know what was done inside, you need to mock the print or stdout and verify how the output message would look like with an assert.
| print(output) | ||
|
|
||
|
|
||
| def fetch_new_data(api_key: str, location_coords: Tuple[float, float]) -> json: |
There was a problem hiding this comment.
You have two choices:
- Make the function an internal function by starting with an underscore
_ - Write a more complete docstring describing the function including the parameters and return value
2 participants
A few unittests to start things off. Main focus is on how to work with Github secrets for the API key.