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

Adds basic testing and fixes includes with more than one argument #692

Merged
merged 7 commits into from
Apr 23, 2014

Conversation

rebelzach
Copy link
Member

  • Support for running npm test "out of the box"
  • A few tests for fixing an issue that came up in the last release regarding live preview includes
  • Brought back PhantomJS

Let me know how the directory structure for /test/ looks, wasn't sure of the best way to lay it out.

I'll probably merge this soon just to get the include bug fixed.

@mikemorris
Copy link
Contributor

Yes yes yes to tests! I'll check out the layout tomorrow but I'd be excited to get this merged.

@tristen
Copy link
Member

tristen commented Apr 23, 2014

yess!! awesome @rebelzach

@mikemorris
Copy link
Contributor

@rebelzach I'd somehow combine polyfill.js, spec and support into a single directory and rename data to fixtures. Aside from that, maybe rename tests.js to index.js? I probably have something misconfigured, but I'm still seeing bind missing from PhantomJS.

@rebelzach
Copy link
Member Author

@mikemorris You may have to run npm install to get the bind replacement module?

@mikemorris
Copy link
Contributor

@rebelzach Ran npm install when I pulled down your branch but still hitting TypeError: 'undefined' is not a function (near '...}).bind(this));...')

@mikemorris
Copy link
Contributor

Oops, I missed that that spec/vendor/liquid.patch.js was actually a test, ignore the comment to merge that into a single directory.

@mikemorris
Copy link
Contributor

@rebelzach Oops, had a blank file at lib/polyfill.js from running make before npm install. make clean && make fixed it, looks good now!

@rebelzach
Copy link
Member Author

@mikemorris I'm wondering if test mocks should just live next the the test. Here is the structure I'm looking at

├── fixtures
│   └── data.js
├── index.html
├── index.js
├── lib
│   ├── chai.js
│   ├── mocha.css
│   ├── mocha.js
│   ├── polyfill-require.js
│   ├── polyfill.js
│   └── tests.js
└── spec
    └── vendor
        ├── liquid.patch.js
        └── liquid.patch.mockFiles.js

@mikemorris
Copy link
Contributor

@rebelzach That looks like a pretty solid structure to me.

@rebelzach
Copy link
Member Author

All right I think its ready to go. I also added source map output for prose.js and the tests. Hopefully make debugging a little easier.

@mikemorris
Copy link
Contributor

@rebelzach Sounds great!

rebelzach added a commit that referenced this pull request Apr 23, 2014
Adds basic testing and fixes includes with more than one argument. Generates source maps for prose.js and test/index.js
@rebelzach rebelzach merged commit b6c8dba into master Apr 23, 2014
@rebelzach rebelzach deleted the fix-include-variables branch April 23, 2014 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants