Skip to content

Change the return type of create_mapping() from MappingResponse to Mapping#99

Merged
oleg-nenashev merged 1 commit intowiremock:masterfrom
hassomehide:fix-create-mapping-response
Dec 12, 2023
Merged

Change the return type of create_mapping() from MappingResponse to Mapping#99
oleg-nenashev merged 1 commit intowiremock:masterfrom
hassomehide:fix-create-mapping-response

Conversation

@hassomehide
Copy link
Contributor

I am not too familiar with this library or WireMock, however I need to create a mapping and remember its ID so I can delete it later. It turns out that the object returned by create_mapping is a MappingResponse full of None values and there is no id property at all.

This PR changes the return type to Mapping, which seems to be what the WireMock server actually returns. MappingResponse is just the type of the response property in Mapping.

The existing unit test did not catch the bug because the mocked response was already wrong to begin with. Weirdly enough this bug has existed since the initial commit.

I did not add any integration tests because I could not get them to work. Integration tests are also failing in CI.

…onse which despite its name is not the correct type
@AlexShimmy
Copy link

Who is responsible for PRs merging? Nice fix, I've faced with the same issue

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

Thanks for catching!

@oleg-nenashev
Copy link
Member

Sorry for the delay, there was no codeowner on this repo so I missed the PRs submitted while I was on the sick leave

@oleg-nenashev oleg-nenashev changed the title Change the return type of create_mapping from MappingResponse to Mapping Change the return type of create_mapping() from MappingResponse to Mapping Dec 12, 2023
@oleg-nenashev oleg-nenashev merged commit c304c06 into wiremock:master Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants