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

unit test for the new homepage #4

Open
wants to merge 1 commit into
base: issue-856-new-homepage
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions pages/home.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,15 @@ var BlogSection = React.createClass({
featuredPost: {
title: "",
author: "",
publishedDate: "",
publishedDate: new Date(),
contentSnippet: "",
link: ""
link: "https://blog.webmaker.org"
},
latestPosts: []
latestPosts: [{
title: "",
publishedDate: new Date(),
link: "https://blog.webmaker.org"
}]
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

have to set some initial values so we don't run into validation / parsing errors during the initial render

Choose a reason for hiding this comment

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

cool, this will be nice for non-JS users too maybe, so that at least there will be a link to the blog they can click on?

},
componentDidMount: function() {
Expand All @@ -109,7 +113,7 @@ var BlogSection = React.createClass({
return;
}
this.setState({
featuredPost: data.featuredPosts,
featuredPost: data.featuredPost,
Copy link
Owner Author

Choose a reason for hiding this comment

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

um sorry for being picky, i just thought this should be singular instead of plural 🙊

Choose a reason for hiding this comment

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

oh yeah totally, that was my bad, thanks for fixing!

latestPosts: data.latestPosts
});
}.bind(this));
Expand Down Expand Up @@ -137,6 +141,9 @@ var BlogSection = React.createClass({
});

var HomePage = React.createClass({
statics: {
BlogSection: BlogSection
},
render: function() {
return (
<div>
Expand Down
30 changes: 30 additions & 0 deletions test/browser/home-page.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
var should = require('should');
var React = require('react/addons');
var TestUtils = React.addons.TestUtils;

var stubContext = require('./stub-context.jsx');
var stubBlogFeedLoader = require('./stub-blog-feed-loader.js');
var HomePage = require('../../pages/home.jsx');

describe("HomePage", function() {
var homePage, blogSection;

beforeEach(function() {
homePage = stubContext.render(HomePage);
blogSection = stubContext.render(HomePage.BlogSection);
});

describe("blogSection", function() {
it("should display featured post", function() {
blogSection.getDOMNode().querySelector(".featured-post .entry-title").textContent.should.not.eql("");
});
it("should display 3 other latest posts", function() {
blogSection.getDOMNode().querySelectorAll(".recent-posts .post-title").length.should.equal(3);
});
Copy link
Owner Author

Choose a reason for hiding this comment

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

Hey @toolness!

( I created this PR against my own PR so that I(or we) can easily compare the code changes. )
I'm having some tough time writing tests for this page. The above 2 test assertions are failing and I can't come up with a solution. Hellllp 😖 Is it because this test gets run before the actual feed data arrives?

Choose a reason for hiding this comment

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

Ahh yeah yup that's what's going on. :( for now i recommend just adding another line at line 16:

// Wait for the stub blog posts to load.
beforeEach(process.nextTick);

It's not super clean--ideally we'd maybe use sinon to trigger the feed callback ourselves--but it'll do for now, hopefully. Let me know if it doesn't work!

Choose a reason for hiding this comment

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

Actually, rather than doing that, you could even wrap that test in the process.nextTick call. Then you could write another test that verifies the blogSection renders ok before it's received the data, too.

Copy link
Owner Author

Choose a reason for hiding this comment

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

@toolness I tried the following and got errors

describe("blogSection", function() {
  process.tick(function() {
    it("should display featured post", function() {
      blogSection.getDOMNode().querySelector(".featured-post .entry-title").textContent.should.not.eql("");
    });
    it("should display 3 other latest posts", function() {
      blogSection.getDOMNode().querySelectorAll(".recent-posts .post-title").length.should.equal(3);
    });
  });
});

screen shot 2015-05-28 at 11 48 43 am

I even tried commenting out stubContext.unmount code but it didn't help. whyyyyyyyyy

Copy link
Owner Author

Choose a reason for hiding this comment

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

I thought it would go like this

  1. create and mount blogSection
  2. wait for feed data to come
  3. test to see blogSection displays feeds
  4. unmount

Choose a reason for hiding this comment

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

Oh, you don't want to do anything asynchronous within describe()--only within it(), and when you do that, you want to pass it a callback that takes a done argument. If you don't do that, mocha will think your test is synchronous, and start tearing down (unmounting) your test setup before the real part of the test has even started!

});

afterEach(function() {
stubContext.unmount(homePage);
stubContext.unmount(blogSection);
});
});
2 changes: 1 addition & 1 deletion test/browser/stub-blog-feed-loader.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
var FAKE_POSTS = {
"featuredPosts": {
"featuredPost": {
Copy link
Owner Author

Choose a reason for hiding this comment

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

"title": "What’s next for Thimble?",
"author": "Hannah Kane",
"publishedDate": "Tue, 12 May 2015 10:47:33 -0700",
Expand Down