Skip to content

fix: update test_geonames_query to assert coordinates as a dict with …#3

Merged
jairomelo merged 1 commit intomainfrom
dev/0.2.2
Jul 15, 2025
Merged

fix: update test_geonames_query to assert coordinates as a dict with …#3
jairomelo merged 1 commit intomainfrom
dev/0.2.2

Conversation

@jairomelo
Copy link
Owner

…latitude and longitude

@jairomelo jairomelo requested a review from Copilot July 15, 2025 00:23
@jairomelo jairomelo merged commit b7935a1 into main Jul 15, 2025
1 check passed
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the test_geonames_query to assert that resolver.resolve returns a dictionary with latitude and longitude keys and validates the expected coordinates for New York, US.

  • Changed return type assertion from tuple to dict
  • Added key presence checks for latitude and longitude
  • Added strict value comparison for the expected coordinates
Comments suppressed due to low confidence (1)

tests/test_resolver.py:19

  • [nitpick] The numbering prefix '2.' in the assertion message is unnecessary and may clutter output; remove it for clearer error messages.
    assert isinstance(coordinates, dict), "2. Coordinates should be a dict"

Comment on lines +18 to +21
assert coordinates is not None, "1. Coordinates should not be None"
assert isinstance(coordinates, dict), "2. Coordinates should be a dict"
assert "latitude" in coordinates and "longitude" in coordinates, "3. Coordinates should contain latitude and longitude"
assert coordinates["latitude"] == 40.71427 and coordinates["longitude"] == -74.00597, f"4. Coordinates {coordinates} do not match expected values for New York, US" No newline at end of file
Copy link

Copilot AI Jul 15, 2025

Choose a reason for hiding this comment

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

[nitpick] The explicit None check may be redundant as subsequent assertions will fail on None; consider removing it to simplify the test.

Suggested change
assert coordinates is not None, "1. Coordinates should not be None"
assert isinstance(coordinates, dict), "2. Coordinates should be a dict"
assert "latitude" in coordinates and "longitude" in coordinates, "3. Coordinates should contain latitude and longitude"
assert coordinates["latitude"] == 40.71427 and coordinates["longitude"] == -74.00597, f"4. Coordinates {coordinates} do not match expected values for New York, US"
assert isinstance(coordinates, dict), "1. Coordinates should be a dict"
assert "latitude" in coordinates and "longitude" in coordinates, "2. Coordinates should contain latitude and longitude"
assert coordinates["latitude"] == 40.71427 and coordinates["longitude"] == -74.00597, f"3. Coordinates {coordinates} do not match expected values for New York, US"

Copilot uses AI. Check for mistakes.
assert coordinates is not None, "1. Coordinates should not be None"
assert isinstance(coordinates, dict), "2. Coordinates should be a dict"
assert "latitude" in coordinates and "longitude" in coordinates, "3. Coordinates should contain latitude and longitude"
assert coordinates["latitude"] == 40.71427 and coordinates["longitude"] == -74.00597, f"4. Coordinates {coordinates} do not match expected values for New York, US" No newline at end of file
Copy link

Copilot AI Jul 15, 2025

Choose a reason for hiding this comment

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

Directly comparing floats can lead to flaky tests; consider using pytest.approx for approximate comparisons of latitude and longitude values.

Copilot uses AI. Check for mistakes.
@jairomelo jairomelo deleted the dev/0.2.2 branch July 15, 2025 02:49
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