-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Issue 4359 pretty print response #7634
Changes from 9 commits
3edcbbc
e7e726c
bfb63d3
271a07b
9770a5c
e043d27
eba009c
29bc192
59a8e4c
2408db5
66bf173
48b1d5b
10b44be
9ee1620
4049e9c
3bc750c
31b688c
bcbca2d
714c94d
337b432
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ import { | |
GraphQLSchema, | ||
GraphQLString, | ||
type ValidationContext, | ||
type FormattedExecutionResult, | ||
} from 'graphql'; | ||
import gql from 'graphql-tag'; | ||
import { | ||
|
@@ -2060,6 +2061,31 @@ export function defineIntegrationTestSuiteHttpServerTests( | |
expect(res.body.errors[0].message).toEqual(expected); | ||
}); | ||
}); | ||
|
||
it('uses stringifyResult parameter', async () => { | ||
const app = await createApp({ | ||
schema, | ||
stringifyResult: (value: FormattedExecutionResult) => { | ||
let result = JSON.stringify(value, null, 1); // stringify, with line-breaks and indents | ||
result = result.replace('it works', 'stringifyResults works!'); // replace text with something custom | ||
return result; | ||
}, | ||
}); | ||
const expected = { | ||
testString: 'stringifyResults works!', | ||
}; | ||
const query = { | ||
query: 'query test{ testString }', | ||
}; | ||
const req = request(app) | ||
.get('/') | ||
.set('apollo-require-preflight', 't') | ||
.query(query); | ||
return req.then((res) => { | ||
expect(res.status).toEqual(200); | ||
expect(res.body.data).toEqual(expected); | ||
}); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is specific to Apollo Server and doesn't necessarily need to be tested for all integrations. I think this might belong in There seems to be a couple layers in testing this. Your Sidenote: |
||
}); | ||
|
||
if (options.serverIsStartedInBackground) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -260,9 +260,11 @@ export async function runHttpQuery<TContext extends BaseContext>({ | |
...graphQLResponse.http, | ||
body: { | ||
kind: 'complete', | ||
string: prettyJSONStringify( | ||
orderExecutionResultFields(graphQLResponse.body.singleResult), | ||
), | ||
string: internals.stringifyResult | ||
? internals.stringifyResult(graphQLResponse.body.singleResult) | ||
: prettyJSONStringify( | ||
orderExecutionResultFields(graphQLResponse.body.singleResult), | ||
), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry I didn't catch this earlier. I'd like to preserve the
string: internals.stringifyResult(
orderExecutionResultFields(
graphQLResponse.body.singleResult,
),
) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense to me -- making change now There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Take a peak? :) |
||
}, | ||
}; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can just remove this test altogether with the existence of the other test. It looks like there are a handful of snapshot files that were generated due to using
toMatchSnapshot
instead oftoMatchInlineSnapshot
- we can delete those too.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on it.