Skip to content

test: Mock resolve_machine_ip() to compensate for FakeServerInfo missing attr - #140

Closed
slothkong wants to merge 1 commit into
mainfrom
test-ip-resolution-mocking-for-pre-delete-server
Closed

test: Mock resolve_machine_ip() to compensate for FakeServerInfo missing attr#140
slothkong wants to merge 1 commit into
mainfrom
test-ip-resolution-mocking-for-pre-delete-server

Conversation

@slothkong

@slothkong slothkong commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Hey @pacospace,
The feature introduced in #138 brought in also a few broken unittests:

FAILED ewccli/tests/ewccli_infra_test.py::test_pre_delete_no_volumes - assert 1 == 0
FAILED ewccli/tests/ewccli_infra_test.py::test_pre_delete_non_ewccli_volumes - assert 1 == 0
FAILED ewccli/tests/ewccli_infra_test.py::test_pre_delete_ewccli_volume_detach_delete - assert 1 == 0
FAILED ewccli/tests/ewccli_infra_test.py::test_pre_delete_detach_failure - assert 'detach/delete failed' in "Unexpected error: 'FakeServerInfo' object has no attribute 'get'"

This PR addresses those 4 particular assertion failures.

What is new

  • Added a mock for resolve_machine_ip() directly in the four tests so they only exercise the volume-detach procedure you care about.

Technical Note

FakeServerInfo doesn't totally mock the interface that pre_delete_server/resolve_machine_ip expect from server_info; it doesnt have a .get() method like the expected dict does.

A solution for the unittests would have also been to force FakeServerInfo to have a get() method, but that ties the tests to the implementation details of the code, which in general not a good practice we can avoid in this case.

@slothkong
slothkong requested a review from pacospace July 31, 2026 08:08
@pacospace

Copy link
Copy Markdown
Member

Thanks for this @slothkong, but this #142 covered also the issue you opened, so I think we can close this.

@slothkong slothkong closed this Jul 31, 2026
@slothkong
slothkong deleted the test-ip-resolution-mocking-for-pre-delete-server branch July 31, 2026 08:57
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