google_maps icon indicating copy to clipboard operation
google_maps copied to clipboard

configurable url, use bypass for api-tests

Open ReneKl opened this issue 5 years ago • 3 comments

make url configurable for api-tests with bypass lib, timezone tests could not be fixed (also in doctest)

ReneKl avatar May 16 '19 10:05 ReneKl

Hi there,

First, thank you for contributing! I really appreciate your efforts into it.

However, I do notice you have been experiencing issues getting the tests to work implementing this PR. Do you think it would be better to raise an issue with what you are encountering, and we can work out the details. Or would you like me to review this PR now?

sntran avatar May 16 '19 15:05 sntran

@sntran Seems like the tests for this one are now passing. Any chance we could get this one merged?

nbw avatar Dec 06 '19 11:12 nbw

Hi @nbw .

With the lack of communication from @ReneKl , it's hard for me to figure out what this PR does. I have spent some time looking through the changes, and here are a few concerns I have:

  • The PR is based on the dev branch, which switched to mint from httpoison. The PR then changed from mint back to httpoision. This is hard for me to merge.
  • Pardon my ignorance, but I don't fully understand the purpose of bypass. If you are to mock API response to your desired, then how the tests would correctly corresponding to actual API response? Yes, the tests will pass, but the library will fail if Google's response is difference.
  • The PR also commented out some tests that do not pass for itself. If the purpose of the PR is to provide more test cases, removing other test cases are not ideal.

sntran avatar Dec 06 '19 18:12 sntran