Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[newman-reporter-allure] Fix: Resolved the issue of formatting in JSON response content displaying in Allure reports. #808

Closed
wants to merge 3 commits into from

Conversation

win5923
Copy link

@win5923 win5923 commented Nov 17, 2023

Context
When the Content-Type of a response body is application/json, the formatting should be normal.

Screenshot
Postman:
image
Postman Response body:
image

Allure report (anonymised for privacy):
image

Checklist

@CLAassistant
Copy link

CLAassistant commented Nov 17, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@epszaw epszaw left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution, good job here!
Please, provide unit-tests for the changes to continue with the PR.

const attachment = this.allureRuntime.writeAttachment(response.body, {
contentType: ContentType.TEXT,
contentType: contentType,
Copy link
Member

Choose a reason for hiding this comment

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

Just leave the property without shadow:

{ contentType }

let contentType = ContentType.TEXT;

// Check if the response content type is JSON
const contentTypeHeader = args.item.responses.members[0]?.getHeader('Content-Type');
Copy link
Member

Choose a reason for hiding this comment

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

Set empty string as fallback value to eliminate nullish checks


// Check if the response content type is JSON
const contentTypeHeader = args.item.responses.members[0]?.getHeader('Content-Type');
if (contentTypeHeader && contentTypeHeader.toLowerCase().includes('application/json')) {
Copy link
Member

Choose a reason for hiding this comment

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

It's ok, but imo, better to use regexp:

/application\/json/i.test(contentTypeHeader)

@epszaw epszaw added the type:bug Something isn't working label Nov 23, 2023
@win5923 win5923 closed this Nov 29, 2023
@win5923 win5923 reopened this Nov 29, 2023
@win5923 win5923 force-pushed the master branch 2 times, most recently from 66e46a4 to 92daa86 Compare November 29, 2023 13:19
@win5923
Copy link
Author

win5923 commented Dec 18, 2023

@epszaw Is there anything else that needs to be modified?

edit: Correcting some typo errors.

@epszaw
Copy link
Member

epszaw commented Dec 19, 2023

@win5923 please, check typescript errors and linters

@win5923 win5923 force-pushed the master branch 5 times, most recently from fc3ad9a to 5616d17 Compare December 20, 2023 01:03
@win5923
Copy link
Author

win5923 commented Dec 21, 2023

Fixed Property 'response' does not exist on type '{ item: Item; }'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme:newman type:bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants