more permissive check for json to include application/vnd.api+j…

[jazwiecki]

Decided to go with the smallest possible change rather than introducing a new package (type-is) as suggested in the neighboring TODO.

Closes #5165.

Pre-merge Tasks

• Have tests been added/updated for the changes in this PR?

[CLAassistant]

All committers have signed the CLA.

[flotwig]

There's a test for the existing application/json behavior here, you can add a test for the application/vnd.api+json behavior as well:

cypress/packages/server/test/unit/request_spec.coffee

Lines 438 to 450 in 8a9ec90

	it "parses response body as json if content-type application/json response headers", ->
	nock("http://localhost:8080")
	.get("/status.json")
	.reply(200, JSON.stringify({status: "ok"}), {
	"Content-Type": "application/json"
	})
	request.sendPromise({}, @fn, {
	url: "http://localhost:8080/status.json"
	cookies: false
	})
	.then (resp) ->
	expect(resp.body).to.deep.eq({status: "ok"})

[flotwig]

Also, I didn't know there was this much controversy about what the correct JSON MIME type is:

• https://stackoverflow.com/questions/477816/what-is-the-correct-json-content-type
• https://stackoverflow.com/questions/28055526/header-value-application-vnd-apijson

[jjnypr]

@flotwig thank you! I can't tell why some of the tests on this are failing. Specifically, server-e2e-tests 2 through 6. At a glance, it doesn't look like the failures have anything to do with swapping includes for endsWith. Does anyone know why this is happening?

[flotwig]

Looking at 3_user_agent_spec, there's this failure: CypressError: Timed out retrying: expected '{"userAgent":"foo bar baz agent"}' to deeply equal { userAgent: 'foo bar baz agent' }

It seems like it's no longer converting JSON according to the same rules, although I'm not sure exactly how your changes caused this, since it looks correct to me.

[jazwiecki]

@flotwig Poking around more, I see some other "deeply equal" errors that seem to hinge on seemingly trivial formatting differences for specs that pass when not run e2e. For example, 2_cookies_spec.coffee returns this error:

  1) cookies without whitelist sends cookies to localhost:2121:
     CypressError: Timed out retrying: expected '{"asdf":"jkl"}' to deeply equal { asdf: 'jkl' }

...but it's just running cookies_spec.coffee, which otherwise works in tests? I'm stumped!

[jazwiecki]

@flotwig I'm a bit embarrassed by this solution but all the tests pass. I don't really know coffeescript so if there's a more elegant way to do this without causing all the tests to fail, I'd love to know more. As is, I've added a test for responses with a content-type of vnd.api+json. OK to merge?

[flotwig]

Got it figured out by setting DEBUG=cypress:server:request and checking the logs - tests were sending content-type': 'application/json; charset=utf-8, which would fail the endsWith check.

Re: the e2e tests - 2_cookies_spec runs a different cookies_spec than the cookies_spec in driver. E2E tests are set up to create a specific environment for the tests (backend server, specific environment setting, certain browser, etc) to test specific conditions.

And "seem to hinge on seemingly trivial formatting differences for specs that pass" - the reason you see a formatting difference, is that it's expecting a JS object, but it's getting a JS string (JSON). So it might look like just a small formatting difference in the error, but it's actually a completely different object.

[flotwig]

LGTM once tests pass