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

Testing for redirects behaves differently in local tests vs. CI #4

Closed
jonnygreen opened this issue Aug 30, 2018 · 5 comments
Closed

Comments

@jonnygreen
Copy link

I have a route that redirects i.e. there is a replaceWith call in the redirect hook.

In local testing, when calling visit on the redirecting URL, the status code returned is 200, and the DOM returned is that of the final, redirected page.

However, when running the same test on CI (Circle), the status code returned is 307 (temporary redirect) and the DOM appears to be the original, non-redirected page.

In other words, it appears that visit on CI is returning the original page, whilst local tests are returning the final redirected page.

This may not be an issue with the add-on, and in fact, given the purpose of the add-on is to test the first render, it may be beyond its scope to test this case in this way. Though the differing results between local and CI seem odd.

If this is expected behaviour, I'd be grateful for any pointers as to why, and how to test for this case.

@ryanto
Copy link
Member

ryanto commented Aug 31, 2018

Thanks for the report!

Something feels off if local and CI have two different results. We've got no tests in here for redirects, so I think a first step is getting these status codes tested in this addon.

Do you mind sharing how you're doing redirects in your app?

@jonnygreen
Copy link
Author

Sure, the route uses the redirect hook...

// app/routes/posts/index.js
export default Route.extend({

    ...

    redirect(model) {
        this.replaceWith('posts', model.firstPostId);
    }
});

The test is then run against the posts index route...

// tests/fastboot/index-redirect-test.js
test('it redirects to the first post', async function(assert) {
    let { htmlDocument, statusCode } = await visit('/posts/');
    assert.equal(statusCode, 200); //returns 307 in Circle CI

    ...
});

Let me know if I can help more.

@ryanto ryanto mentioned this issue Sep 4, 2018
@ryanto
Copy link
Member

ryanto commented Sep 4, 2018

Awesome, thanks!

I did a little digging and it looks like fastboot responding with 307 for replaceWith is the correct behavior. Here's the test case in the FastBoot repo: https://github.com/ember-fastboot/ember-cli-fastboot/blob/master/test/fastboot-location-test.js#L94-L115

Just to be safe, I added some redirect tests into this addon's test suite as well: #8

If your local environment and CI are having different results you might want to confirm that both environments are running the same version of FastBoot.

If it's still causes issues for you, do you mind creating a new repo where we can see this bug in isolation? That way we can see what it will take to fix this upstream in fastboot.

@ryanto ryanto closed this as completed Sep 4, 2018
@ryanto
Copy link
Member

ryanto commented Sep 4, 2018

Closing for now, but feel free to reopen!

@jonnygreen
Copy link
Author

Thanks Ryan

I've tried running the test suite for ember-cli-fastboot-testing locally. I now get the same redirect test failures as I was experiencing in my own app i.e. running the test locally for me actually performs the redirect and then runs the test on the resulting page:

screen shot 2018-09-05 at 11 26 09

Obviously, I'm running the same versions as specified in the package.json, so the only variable I can think remaining is node - could this be a node version issue? Locally, I'm running 8.9.1. My CI tests are running 8, though I can't discover the point version.

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