cfpb / regulations-site

(DEPRECATED) Web interface for viewing U.S. federal regulations and other regulatory information
Other
28 stars 43 forks source link

Refactoring tests #696

Closed KimberlyMunoz closed 9 years ago

KimberlyMunoz commented 9 years ago

Definitely want a second or third pair of 👀 on these.

Using Owning A Home as an example, I replaced mocha with mocha_instanbul to give us coverage statistics and wrote tests for more functions in Helpers.js.

Other Changes

ascott1 commented 9 years ago

:heart: this. I can't wait to dig in.

ascott1 commented 9 years ago

@KimberlyMunoz @imuchnik: I am running the app, pulled down @KimberlyMunoz's branch, and ran npm install, but when I try to run grunt mocha_istanbul I get:

0 passing (364ms)
1 pending
2 failing

The errors seem related to a "before all" hook.

Did I miss a step?

Thanks!

KimberlyMunoz commented 9 years ago

I just deleted my node_modules folder and ran npm install and didn't get that error. I'm going to check to see if there's a missing file somewhere that didn't get added to this pull request.

ascott1 commented 9 years ago

Whoops. Looks like I was running an old version of Node*. Using Node 0.12.0 seems to solve that problem.

*This recommends adding source $(brew --prefix nvm)/nvm.sh to your .bash_profile when installing nvm via homebrew. Apparently nvm was not running for me after a recent machine reboot.

ascott1 commented 9 years ago

This seems to cause issues with routing for me:

screen shot 2015-03-23 at 4 16 15 pm

That URL should be something more like partial/1005-3/2014-20681. If I had to guess, I'd say it's related to the change on this line, but I haven't dug in more closely yet.

KimberlyMunoz commented 9 years ago

🔨 That should fix it.

ascott1 commented 9 years ago

This is great and works beautifully. I have one comment above about the naming of the resources file, but once we talk through that, this is ready to merge. Nice work @KimberlyMunoz! And thanks for your help @imuchnik & @contolini!