Fixes #953 - homepage unit test

[mmmavis]

This fixes #953

[mmmavis]

@toolness in this comment you mentioned about using sinon to trigger the feed callback. Could you give me more pointers?

[toolness]

Hmm, so for now I think what you've got is actually ok, so long as you add a comment above line 18 that explains how the process.nextTick is used so the stub blog post loader loads the posts by then.

However, if you do want to go the extra mile/kilometer, that's totally awesome! It'd likely involve the following steps:

1. Writing a fake loadBlogPosts function that you pass as a prop to HomePage.BlogSection. You could use a sinon spy for this, or you could actually just hand-write a fake function that looks something like this:

   var respondWithBlogPosts;
   var fakeLoadBlogPosts = function(cb) {
     /* Hold on to the callback so we can trigger it later from our tests. */
     respondWithBlogPosts = cb;
   };

2. You might want to change stub-blog-feed-loader.js so it also exports FAKE_POSTS. You'll likely want your fake callback to pass this back to the component.

That's the basics... Writing tests is mostly just about "hacking" the inputs/dependencies of your code to make sure they provide the expected output, and preferably doing so in a way that's easy to understand and maintain. Feel free to experiment with other ideas if you don't like the ones I provided.

[toolness]

Could you add at least one test that verifies how the component should behave when it's waiting for blog posts to load? You can even fix #955 real quick while you're at it if you want 😁

[mmmavis]

@toolness I have this describe inside of the describe("HomePage", does it help solving the issue you mentioned? Or I could remove this inner one and rename the outer describe from HomePage to HomePage.BlogSection

[toolness]

Yeah, I recommend just removing the inner one and renaming the outer one--that way we can keep each test suite completely isolated from one another.

The only reason we might want to nest describe calls is if two components or "types" of test suites need to share common infrastructure--in which case the beforeEach/afterEach of the top-level describe could do the setup/teardown common to both sub-suites. But until we definitely have a need for that, I think we should just keep the different suites isolated from one another as much as possible. (Unfortunately, I don't think all the test suites I've written follow this principle--sorry I'm not practicing what I'm preaching... 😞 )

[mmmavis]

@toolness I updated this PR again (with potential fix for #955 as well). please review and let me know if it looks sane 🐹

[mmmavis]

oh yea don't forget the ?w=1 trick 😁 https://github.com/mozilla/teach.webmaker.org/pull/992/files?w=1

[toolness]

For non-JS users and search engines, I wonder if it might be useful to at least link to the blog instead of not showing anything at all? Guess we can just file an issue for it--and anyways, once we move to the dynamic lightweight server in #585, we will be able to fetch the blog info on the server side and deliver this component completely pre-rendered, so not really a big deal I guess.

[mmmavis]

Before the data has been loaded from Google API we don't have the any info about the latest blog posts, do we? 😮 On the initial render we are just passing https://github.com/mozilla/teach.webmaker.org/blob/develop/pages/home.jsx#L94-L105 as props to <FeaturedPost>.

Both of the <FeaturedPost> and <LatestPosts> won't show but there will be a "See all blog posts" link that links to http://blog.webmaker.org/. I hope that's sufficient 👀

[toolness]

Ack, I'm realizing that there are a few comments I should've made on your original PR for the blog post components:

• In FeaturedPost, I recommend providing the data fields (like publishedDate, contentSnippet, etc) directly as props rather than in a single nested data object. In fact, whenever possible, we should probably prefer to not pass nested data structures into props, as it makes our code more self-documenting (if we use propTypes), and it makes it really easy for us to use PureRenderMixin to speed up our site. (That mixin only compares top-level props and doesn't dig down into sub-objects and sub-arrays.)

  Oh, also, when you use the FeaturedPost component in BlogSection, you can easily pass this.state.featuredPost in as a set of props by using JSX Spread Attributes. (Or you can just pass the props in explicitly, it's up to you.)

• You mentioned a little while ago that you tend to use the word data too much, so I'm calling you out on it here! 😛 Eliminating props.data for FeaturedPost as described above would be great. And in LatestPosts, you might want to consider renaming props.data to props.posts, as that's a more informative name.

Anyhow, neither of these are blockers or anything, but if you want to fix them in this PR that'd be rad, or we can file a separate issue for them.

[mmmavis]

Oh, also, when you use the FeaturedPost component in BlogSection, you can easily pass this.state.featuredPost in as a set of props by using JSX Spread Attributes. (Or you can just pass the props in explicitly, it's up to you.)

raaaad, now I know another thing to save my keystrokes!

review again @toolness? 😬

[toolness]

Woot, this looks GREAT! Thanks mavis!