Skip to content

Conversation

manuel1618
Copy link
Contributor

A few unittests to start things off. Main focus is on how to work with Github secrets for the API key.

Copy link
Contributor

@codie3611 codie3611 left a comment

Choose a reason for hiding this comment

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

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}}
Copy link
Contributor

Choose a reason for hiding this comment

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

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
Copy link
Contributor

Choose a reason for hiding this comment

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

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
Copy link
Contributor

Choose a reason for hiding this comment

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

Another newline 👀

cmds:
- >
poetry run pytest
-s
Copy link
Contributor

Choose a reason for hiding this comment

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

Pleas write out command line options if you can. I have no fricking clue what -s is ❓

Comment on lines +10 to +12
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
Copy link
Contributor

Choose a reason for hiding this comment

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

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)
Copy link
Contributor

Choose a reason for hiding this comment

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

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 = {
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Comment on lines +56 to +58
if "TNT_EX1_OPENWEATHERMAP_API_KEY" not in os.environ:
dotenv_file = dotenv.find_dotenv()
dotenv.load_dotenv(dotenv_file)
Copy link
Contributor

Choose a reason for hiding this comment

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

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))
Copy link
Contributor

Choose a reason for hiding this comment

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

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:
Copy link
Contributor

Choose a reason for hiding this comment

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

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

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