Skip to content

Issue 4 currency converter#12

Open
Xarvius wants to merge 28 commits intomasterfrom
issue-4-currency-converter
Open

Issue 4 currency converter#12
Xarvius wants to merge 28 commits intomasterfrom
issue-4-currency-converter

Conversation

@Xarvius
Copy link
Owner

@Xarvius Xarvius commented Sep 10, 2019

No description provided.

@Xarvius Xarvius mentioned this pull request Sep 10, 2019
Copy link
Collaborator

@noisy noisy left a comment

Choose a reason for hiding this comment

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

Code is more or less ok, but I have a new challenge for you. Please add unit tests (in a separate file) to this program.

Your unittests should document expected responses from API which you are using, Also your tests should tests, whether your program behaves as expected in case of different errors returned by a 3rd party API.

@Xarvius
Copy link
Owner Author

Xarvius commented Sep 11, 2019

bump

param = {
"base": start
}
response = requests.get("https://api.exchangeratesapi.io/latest", params=param)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this tests should test currency_converter, and not make a request to https://api.exchangeratesapi.io/latest API.

You should try to mock a response of this API, by mocking response of requests.get. Here is an example how you can do that: https://realpython.com/testing-third-party-apis-with-mocks/ (don't bother with nose in this article - an alternative python testrunner)

TL;DR; Your test by mocking a response of requests.get will simulate a server https://api.exchangeratesapi.io/latest.

Pros of this approach:

  • This will be an unittest and not an integration test
  • it will "freeze" in a test in some kind of documentation a response of an API, which current implementation support.
  • such unittests will pass on computer being offline. No internet connection is needed.
  • they will be much faster - no latency.

print("Musisz podać liczbę!")
continue
print("Trwa sprawdzanie kursu...")
currency_converter(start_currency, end_currency, amount_currency)
Copy link
Collaborator

Choose a reason for hiding this comment

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

currency_converter returns value, but this value is never used.

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