Skip to content

MultipartFormDataHelperTest - Unit Tests#44807

Closed
markmunozmsft wants to merge 1 commit intoAzure:mainfrom
markmunozmsft:multipart_form_data_unit_test
Closed

MultipartFormDataHelperTest - Unit Tests#44807
markmunozmsft wants to merge 1 commit intoAzure:mainfrom
markmunozmsft:multipart_form_data_unit_test

Conversation

@markmunozmsft
Copy link
Contributor

Unit Tests added to
MultipartFormDataHelperTest

@github-actions
Copy link
Contributor

Thank you for your contribution @markmunozmsft! We will review the pull request and get back to you soon.

@github-actions github-actions bot added Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization. OpenAI labels Mar 27, 2025
@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

return name.replace("\n", "%0A").replace("\r", "%0D").replace("\"", "%22");
}

public String getBoundary() {
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, this class is generated, so next time we run our code generation tools, this method will probably disappear.

Copy link
Member

Choose a reason for hiding this comment

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

But there is really no other way to test this, as we need to know ahead of time the value of the boundary. I think that instead of adding this method, it might be better to make the construtor in line ~65 package private. So changing from:

private MultipartFormDataHelper(RequestOptions requestOptions, String boundary) ->
MultipartFormDataHelper(RequestOptions requestOptions, String boundary)

Copy link
Member

Choose a reason for hiding this comment

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

By doing this you can inject an expect boundary value against which you can assert in your tests.

}

@Test
public void testDeserializeAudioTranscription() throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

I think this test should probably be moved to under the models package, as it doesn't seem to relate to the MultipartFormDataHelper class.

That being said, are we exercising here anything that isn't exercised in the testGetAudioTranscription* tests in the NonAzureOpenAISyncClient and all the other files?

@jpalvarezl
Copy link
Member

jpalvarezl commented Apr 2, 2025

I think that the CI failures relate to add a new public String getBoundary() method without documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization. OpenAI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants