veged / borschik

Main repository moved to https://github.com/bem/borschik. Extendable builder for text-based file formats.
http://bem.info/articles/borschik/
94 stars 0 forks source link

Fix tests #49

Closed alexeyten closed 11 years ago

alexeyten commented 11 years ago

Borschik is async, so we have to wait for before.

arikon commented 11 years ago

@alexeyten Why? We use mocha-as-promised so we can return promises as results of tests functions.

alexeyten commented 11 years ago

I guessmocha-as-promised doesn't convert before-hook into promise.

arikon commented 11 years ago

I'll take a look into it today. Thanks!

alexeyten commented 11 years ago

May be you should get rid of before-hooks and put convertation into it.

arikon commented 11 years ago

I've looked into mocha-as-promised. It should work with hooks also. mocha-as-promised injects into Runnable and Hook inherits from Runnable, so all kinds of hooks could return promises and they should work as expected.

Could you show output of npm ls in borschik dir?

alexeyten commented 11 years ago
~/Public/borschik|master$ npm ls
npm WARN package.json fs-boot@0.0.9 'devDependences' should probably be 'devDependencies'
borschik@0.2.3 /home/lynn/Public/borschik
├── coa@0.3.8
├── coffee-script@1.4.0
├── csso@1.3.6
├── cssp@1.0.6
├── inherit@1.0.4
├─┬ istanbul@0.1.22
│ ├── abbrev@1.0.3
│ ├── async@0.1.22
│ ├─┬ escodegen@0.0.15
│ │ ├── esprima@1.0.2
│ │ └─┬ source-map@0.1.8
│ │   └── amdefine@0.0.4
│ ├── esprima@0.9.9
│ ├─┬ fileset@0.1.3
│ │ ├─┬ glob@3.0.1
│ │ │ ├── fast-list@1.0.2
│ │ │ ├── graceful-fs@1.1.14
│ │ │ └── inherits@1.0.0
│ │ └─┬ minimatch@0.1.5
│ │   └── lru-cache@1.0.6
│ ├─┬ handlebars@1.0.7
│ │ ├── optimist@0.3.5
│ │ └── uglify-js@1.2.6
│ ├── mkdirp@0.3.4
│ ├── nopt@2.0.0
│ ├── which@1.0.5
│ └── wordwrap@0.0.2
├─┬ mocha@1.7.2
│ ├── commander@0.6.1
│ ├── debug@0.7.0
│ ├── diff@1.0.2
│ ├── growl@1.6.1
│ ├─┬ jade@0.26.3
│ │ └── mkdirp@0.3.0
│ ├── mkdirp@0.3.3
│ └── ms@0.3.0
├── mocha-as-promised@1.1.0
├── mocha-istanbul@0.1.0
├── ometajs@3.1.15
├── q@0.8.11
├─┬ q-fs@0.1.36
│ ├── fs-boot@0.0.9
│ └── q-io@0.0.18
└── uglify-js@1.3.4
alexeyten commented 11 years ago
~$ node -v
v0.8.15

node from https://launchpad.net/~chris-lea/+archive/node.js

arikon commented 11 years ago

@alexeyten I double checked, mocha-as-promised works for all hooks and tests.

arikon commented 11 years ago

@alexeyten How do you run tests that they fail? Could you provide command and its output?

alexeyten commented 11 years ago

just run make test https://gist.github.com/cb8edf0506c813d567e3

arikon commented 11 years ago

@alexeyten Could not reproduce =( Just cloned clean copy from GH, npm install && make test, and all works as expected.

Any ideas?

alexeyten commented 11 years ago

Yes. In my installation freeze-test.js runs before common.js. That's because readdirSync returns unordered list of entries.

alexeyten commented 11 years ago

So this pull request is incorrect. I guess you should put common.js into freeze-test.js and everything will be fine.

alexeyten commented 11 years ago

created #50

arikon commented 11 years ago

@alexeyten This should not matter because mocha-as-promised alters Runnable.prototype and tests in fact are run just after all test files have been read and evaled.

arikon commented 11 years ago

Oh, a think I fixed it in 158387de7acf503cc89b23c037b3115cbefa5a0b Could you check?

alexeyten commented 11 years ago

Yes fixed.

Order matters because mocha-as-promised also alters fn property setter and getter.

arikon commented 11 years ago

@alexeyten I've got it. Thanks for your time spent! ;)