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

Add client/server rendering coherency tests #2864

Closed
wants to merge 6 commits into from
Closed

Add client/server rendering coherency tests #2864

wants to merge 6 commits into from

Conversation

levibotelho
Copy link

Added test cases that compare rendering on the client and server (one of which fails intentionally). Hopefully these address the root cause but feel free to weigh in if they're not quite right.

See #2714

@levibotelho
Copy link
Author

See the render to body is causing issues -- I can correct that no problem but would like to know if there are other things to correct/change before I do 😃

@taion
Copy link
Contributor

taion commented Jan 7, 2016

Thanks.

@levibotelho
Copy link
Author

Not sure why it's still failing. Worked perfectly in my dev environment with npm run-script test but this is my first contribution to react-router so I might be missing something obvious. Would appreciate if someone could take a look -- just trying to push #2714 along because it's a pretty annoying bug.

@timdorr
Copy link
Member

timdorr commented Jan 7, 2016

We run through BrowserStack, which tests against different browsers. Those are what's failing.

On MobileSafari for iOS7 is failing for some internal reason ('null' is not an object? Uh... typeof null === 'object'...)
On Chrome 39 on Windows 8.1 with Error: Expected <div id="container">...</div> to equal <div id="container">...</div>. wat.

I think we could stand to update the browser testing config. I'll put in a PR for that and you can rebase against master to get those changes, which will hopefully fix up things here.

@levibotelho
Copy link
Author

Thanks a lot @timdorr ! This worked locally so yeah, not quite sure why it's not working here on these browsers. I had to fiddle a bit with retrieving the DOM nodes etc. to get a coherent comparison but there's nothing too extraordinary going on in my tests either. Let me know when you've done the config update and I'll rebase.

@timdorr
Copy link
Member

timdorr commented Jan 7, 2016

@levibotelho I just merged that in. Can you rebase now?

@levibotelho
Copy link
Author

No luck still :/. One thing I noticed is that the tests are running on Chromium 37 (dates back to June 2014) whereas my dev machine is running Chromium 47. I still don't see why it would fail the test itself is really basic. The only thing that I can think of is maybe the React-specific attributes that I strip out in the test code are slightly different in that version of Chromium.

If there is a way to make the test output more verbose that would be nice. Otherwise I could add an eslint exception to allow console statements and do a console.log in those tests to find the problem and remove it afterwards.

The asynchronous handler test does appear to fail as expected, however.

@levibotelho
Copy link
Author

@timdorr I'm wondering if the test is failing because I'm using DOMParser. It's been fully supported in Chrome since version 30, but Chromium may be a different story?

})

function executeRenderingComparisonTest(routes, callback) {
match({ routes, location: targetLocation }, function (error, redirectLocation, renderProps) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not the correct usage – this should be the set of routes, not the <Router>.

Copy link
Author

Choose a reason for hiding this comment

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

Ah thanks!

@taion
Copy link
Contributor

taion commented Jan 10, 2016

Why is the async onEnter hook test not failing?

@taion
Copy link
Contributor

taion commented Jan 11, 2016

Superseded by #2883.

@taion taion closed this Jan 11, 2016
@levibotelho
Copy link
Author

@taion I don't know. This is the output I get on my local machine.

SUMMARY:
✔ 255 tests completed
ℹ 1 test skipped
✖ 1 test failed

FAILED TESTS:
  RouterContext
  server rendering
    ✖ renders the same output on the server and on the client when there is an asynchronous onEnter handler
      Chromium 47.0.2526 (Ubuntu 0.0.0)
    Uncaught Error: Expected <div id="container">...</div> to equal <noscript></noscript> (/home/levi/Dev/react-router/tests.webpack.js:300 <- webpack:///~/expect/lib/Expectation.js:75:0)
    Error: Uncaught Error: Expected <div id="container">...</div> to equal <noscript></noscript> (/home/levi/Dev/react-router/tests.webpack.js:300 <- webpack:///~/expect/lib/Expectation.js:75:0)

Which seems to reproduce the issue.

If I can lend a hand in any way let me know.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants