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

Include all request headers in Fastboot request? #13

Closed
jkeen opened this issue Nov 2, 2018 · 5 comments
Closed

Include all request headers in Fastboot request? #13

jkeen opened this issue Nov 2, 2018 · 5 comments

Comments

@jkeen
Copy link
Collaborator

jkeen commented Nov 2, 2018

That PR I submitted yesterday (#12) added the user-agent request header, but now I'm realizing I need the cookie header also to test that fastboot can serve pages requiring authentication. I have a PR ready, but before I add another one-off I thought I'd ask if we shouldn't just include all the request headers? Can you think of a reason not to, @ryanto ?

@ryanto
Copy link
Member

ryanto commented Nov 2, 2018

Yes, I think as a default we should pass as much info from the browser into the fastboot request. That will certainly help unexpected bugs like the one you ran into yesterday.

One thing I've sort of been putting off is the ability to configure these headers/options from the test. For example, you might want to write multiple tests with different headers/cookies.

test('my fastboot app renders when a user is logged in', async function(assert) {
  await visit('/', { headers: { cookie: 'some-logged-in-user-cookie' } });
});

But I'm not sure what exactly this API should be. There's a whole bunch of options... everything from the request to how fastboot runs.

So yah... A good starting point would be exactly what you said, include all request headers. Later we can figure out what folks want to change per test. Thoughts?

@jkeen
Copy link
Collaborator Author

jkeen commented Nov 2, 2018

Perfect, we're on the same page. I was thinking the same thing as far that API went. I'll get something worked up and submit that PR.

This is an entirely different topic, but another issue I'm running into related to this whole fastboot testing story is mocking some requests that happen on the fastboot side of things. Where y'all at on your thoughts surrounding the mirage + fastboot strategy? Ideally I'd like these fastboot tests to verify that data is getting loaded and rendered on the server side. Any advice for mocking one request on the fastboot side without mirage?

@ryanto
Copy link
Member

ryanto commented Nov 2, 2018

Awesome 👍

For the testing data I don't have a good answer. In the past I've used Ember CLI's express mocks for this sort of thing: https://github.com/embermap/ember-data-storefront/blob/master/server/mocks/posts.js and https://github.com/embermap/ember-data-storefront/blob/master/server/index.js. It takes a bit of playing around with to get it right.

Sam and I want to get mirage working in node... and once that happens fastboot would be able to talk to it. We need this to get our testing story right, but I think we're still a few months away from getting this working. In my mind that's the best way forward, but it doesn't exist today :)

@jkeen
Copy link
Collaborator Author

jkeen commented Nov 2, 2018

Great, thanks! I'll give that a whirl.

Just submitted #14 which should close this issue, and here's a try at that header overriding thing if that's a direction you think we should take.

@ryanto
Copy link
Member

ryanto commented Nov 3, 2018

Awesome, published as 0.0.6!

The header overriding looks good too!

@ryanto ryanto closed this as completed Nov 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants